Conversation
| if (name.startsWith(SERVICES_PREFIX)) { | ||
| String fqn = name.substring(SERVICES_PREFIX.length()); | ||
| if (fqn.startsWith(OTEL_NAMESPACE)) { | ||
| OtelSpiCollector.getInstance().recordSpiDetected(fqn, EXTENSIONS_PATH_SOURCE); |
There was a problem hiding this comment.
can we exit here do avoid looping on entries if we already found?
There was a problem hiding this comment.
If the goal here is to report all service files under the OTel namespace then the best we could do is exit on the first entry that doesn't start with the services prefix. That's assuming that the service entries appear in a group, which they usually do. If assume that the service entries are ordered then we could exit on the next entry that doesn't start with the OTel namespace.
However I'd like to know about what we intend to gain from this telemetry since this will incur a non-trivial startup cost, albeit only when extensions are added to the tracer (which is uncommon.)
i.e. if the goal is to survey what SPIs are being added as extensions, even though we don't currently support them then I guess this is the only way to discover that (although it would be a good idea to short-circuit the searching when we get to jar entries that we know should appear after service entries - you can look at some example OTel extension jars to see if there's a pattern.)
There was a problem hiding this comment.
the goal is to survey what SPIs are being added as extensions, even though we don't currently support them
This is exactly the goal here. For the purpose of having information of what SPIs are commonly being used even though we don't support them yet.
There was a problem hiding this comment.
EDIT: See comment below. TLDR scanning from a static list instead of iterating through all jars.
@mcculls I had Claude go thru some sample OTel extension jar files from Maven Central and it found that Services entries are contiguous, but custom services entries can be shoved in-between OTel service entries.
e.g.:
[1357] META-INF/services/io.opentelemetry.sdk.autoconfigure.spi.ResourceProvider
[1358] META-INF/services/co.elastic.otel.common.ChainingSpanProcessorAutoConfiguration
[1359] META-INF/services/io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule
Generally, it appears that JAR files will write all service entries contiguously, but that's technically not guarnateed. IMO, some data is better than no data, so I'm fine w/ reading until the end of service entries, and quitting early. I'll implement this change here.
|
@amarziali @mcculls I've updated this PR to use a static list of known OTel SPIs instead of looping through all JAR files. This list is fairly limited and should hopefully reduce the impact on startup time. |
What Does This Do
This PR sends metrics for all tracked OTel SPIs that are registered at any of the JAR files on the path listed by
otel.javaagent.extensionsordd.trace.extensions.path. We use a static list of SPIs in order to reduce the impact on startup time. This list can be updated as OTel increases their support scope.Motivation
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: [PROJ-IDENT]
Note: Once your PR is ready to merge, add it to the merge queue by commenting
/merge./merge -ccancels the queue request./merge -f --reason "reason"skips all merge queue checks; please use this judiciously, as some checks do not run at the PR-level. For more information, see this doc.