-
Notifications
You must be signed in to change notification settings - Fork 1k
Remove AgentInstrumentationConfig usages: AWS #15651
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
0726fec to
4efb7be
Compare
4efb7be to
9fe722b
Compare
ce03df6 to
7fa0ebe
Compare
| * | ||
| * @deprecated Use {@link #setMessagingReceiveTelemetryEnabled(boolean)} instead. | ||
| */ | ||
| @Deprecated | ||
| @CanIgnoreReturnValue | ||
| public AwsSdkTelemetryBuilder setMessagingReceiveInstrumentationEnabled( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
renamed this to match the property
// instrumentation/development:
// java:
// messaging:
// receive_telemetry/development:
// enabled: false
| * @deprecated Use {@link #setMessagingReceiveTelemetryEnabled(boolean)} instead. | ||
| */ | ||
| @Deprecated | ||
| @CanIgnoreReturnValue | ||
| public AwsSdkTelemetryBuilder setMessagingReceiveInstrumentationEnabled( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
| } | ||
| protected abstract boolean recordIndividualHttpError(); | ||
|
|
||
| protected abstract boolean getBoolean(String name, boolean defaultValue); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there used to be this generic getBoolean function here, I've replaced it with individual getters for each of the needed configuration properties
note that this class is odd because it still supports two configuration paths:
- java agent (now via declarative configuration API)
- library (still driven by reading system properties directly)
we still need to migrate library configuration later, at which time we can unify this(!)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's already a PR for this: #15656
|
Closing, going to re-do with (really) minimal changes over to declarative configuration API (including not removing ExperimentalConfig and CommonConfig) |
Depends on #15668