Automatic AWS library instrumentor #4607
Conversation
steven-aerts
requested review from
anuraaga,
iNikem,
jkwatson,
laurit,
mateuszrzeszutek,
pavolloffay,
trask and
tylerbenson
as code owners
|
|
steven-aerts
force-pushed
the
aws-sdk-instrumentor
branch
2 times, most recently
from
16bee72 to
a9e8363
Compare
There was a problem hiding this comment.
Thanks @steven-aerts! Sorry for the huge delay on reviewing
instrumentation/aws-sdk/aws-sdk-1.11/library-instrumentor/build.gradle.kts
Outdated
Show resolved
Hide resolved
instrumentation/aws-sdk/aws-sdk-1.11/library-instrumentor/build.gradle.kts
Outdated
Show resolved
Hide resolved
instrumentation/aws-sdk/aws-sdk-2.2/library-instrumentor/build.gradle.kts
Outdated
Show resolved
Hide resolved
steven-aerts
force-pushed
the
aws-sdk-instrumentor
branch
from
a9e8363 to
cdea623
Compare
|
|
||
| private final OpenTelemetry openTelemetry; | ||
|
|
||
| private boolean captureExperimentalSpanAttributes; | ||
|
|
||
| AwsSdkTracingBuilder(OpenTelemetry openTelemetry) { | ||
| this.openTelemetry = openTelemetry; | ||
| this.captureExperimentalSpanAttributes = CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES_DEFAULT; |
There was a problem hiding this comment.
other library instrumentations only allow programmatic configuration of captureExperimentalSpanAttributes, I'd suggest doing the same here for consistency (setting this from the autoconfigure module), and opening an issue to discuss doing this across all library instrumentations if there's interest
There was a problem hiding this comment.
Hi @trask ,
The captureExperimentalSpanAttributes could already be configured by these system properties/environment variables. I only moved this mechanism to make sure this can be shared between the java-agent and the autoconfigure library.
I understand that the default for the builder are now also changing based on these variables.
Would it be a solution if I add an extra method to the AwsSdkTracingBuilder, which is then called by both the java-agent as the autoconfigure class?
Or do you prefer the PR where I copy this behaviour to all other classes which have these Config variables for the java-agent?
There was a problem hiding this comment.
Would it be a solution if I add an extra method to the
AwsSdkTracingBuilder, which is then called by both the java-agent as the autoconfigure class?
AwsSdkTracingBuilder already has setCaptureExperimentalSpanAttributes, can you call this from both javaagent and autoconfigure modules? I think that would be most consistent with other instrumentations currently.
There was a problem hiding this comment.
@trask I tried to update it, while still keeping the Config parameter in one place for v1.1.
Let me know what you think.
steven-aerts
force-pushed
the
aws-sdk-instrumentor
branch
from
cdea623 to
b84ad1e
Compare
| public static AwsSdkTracing fromAgentConfig(OpenTelemetry openTelemetry) { | ||
| return builder(openTelemetry) | ||
| .setCaptureExperimentalSpanAttributes(CAPTURE_EXPERIMENTAL_SPAN_ATTRIBUTES_DEFAULT) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
I don't think we want to expose "agent" methods in library instrumentation, can the agent call setCaptureExperimentalSpanAttributes directly instead?
There was a problem hiding this comment.
@trask I inlined the function on the two places where it is used.
This currently means that also Config.get().getBoolean("otel.instrumentation.aws-sdk.experimental-span-attributes", false)) is duplicated.
Let me know if you prefer this to be put in a constant on the AwsSdkTracing class or its builder.
There was a problem hiding this comment.
thx! we have some related work to do (and will likely end up where you were going originally with adding Config default directly to library instrumentation) but appreciate keeping it consistent with existing instrumentation for now 👍
Like AWS X-Ray, provide an instrumentor which automatically registers opentelemetry instrumentation in the AWS SDK without any code changes. Those instrumentors are separate libraries published as opentelemetry-aws-sdk-1.11-instrumentor and opentelemetry-aws-sdk-2.2-instrumentor
steven-aerts
force-pushed
the
aws-sdk-instrumentor
branch
from
b84ad1e to
6f3abfd
Compare
Like AWS X-Ray, provide an instrumentor which automatically registers opentelemetry instrumentation in the AWS SDK without any code changes. Those instrumentors are separate libraries published as opentelemetry-aws-sdk-1.11-instrumentor and opentelemetry-aws-sdk-2.2-instrumentor
Like AWS X-Ray, provide an instrumentor which automatically registers
opentelemetry instrumentation in the AWS SDK without any code changes.
Those instrumentors are separate libraries published as
opentelemetry-aws-sdk-1.11-instrumentor and opentelemetry-aws-sdk-2.2-instrumentor