Replace inferred-spans extension with upstream contrib extension #370
Conversation
# Conflicts: # inferred-spans/build.gradle.kts
JonasKunz
force-pushed
the
inferred-spans-upstream
branch
from
a874dbb to
8aa033c
Compare
# Conflicts: # inferred-spans/README.md # inferred-spans/src/main/java/co/elastic/otel/profiler/InferredSpansAutoConfig.java # inferred-spans/src/test/java/co/elastic/otel/InferredSpansAutoConfigTest.java # licenses/more-licences.md
|
Support for |
|
Waiting on open-telemetry/opentelemetry-java-contrib#1533 to implement full backwards compatibility |
# Conflicts: # inferred-spans/README.md # licenses/more-licences.md
# Conflicts: # gradle/libs.versions.toml
|
open-telemetry/opentelemetry-java-contrib#1533 is now part of the included contrib version, so this PR is ready for review |
| * Marker attribute for inferred spans. Does not have the elastic-prefix anymore because it has | ||
| * been contributed upstream | ||
| */ | ||
| AttributeKey<Boolean> IS_INFERRED = AttributeKey.booleanKey("is_inferred"); |
There was a problem hiding this comment.
[minor] do we have a way to reuse the upstream attribute without duplicating it ?
| private SamplingProfiler extractProfiler(InferredSpansProcessor processor) { | ||
| try { | ||
| Field profilerField = processor.getClass().getDeclaredField("profiler"); | ||
| profilerField.setAccessible(true); | ||
| return (SamplingProfiler) profilerField.get(processor); | ||
| } catch (Exception e) { | ||
| throw new RuntimeException(e); | ||
| } | ||
| } |
There was a problem hiding this comment.
[minor] wouldn't it be better to avoid using reflection and rely on a package-private helper class here ?
| .put("elastic.otel.inferred.spans.enabled", "true") | ||
| .put("elastic.otel.inferred.spans.logging.enabled", "false") | ||
| .put("elastic.otel.inferred.spans.backup.diagnostic.files", "true") |
There was a problem hiding this comment.
maybe it would be worth adding a test to test the priority between legacy and upstream config options, focusing only on a few basic ones (and we can assume the same priority is applied to others).
Closes #174.
Replaces the inferred-spans extension with the contributed upstream one, but maintains a backwards compatibility layer for the
ELASTIC_options because they are used a lot already in demos.This PR is also kind of blocked by elastic/apm-data#321. That change did not make it into
8.15. We'll need that change to ship, otherwise the parent-child overriding won't wokr correctly, because the attributeelastic.is_childwas renamed tois_child.