-
Notifications
You must be signed in to change notification settings - Fork 129
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
[maven-extension] Use the Otel SDK Auto Configuration Extension #112
[maven-extension] Use the Otel SDK Auto Configuration Extension #112
Conversation
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 are improvements we can make in autoconfigure to help with the properties stuff indeed
|
||
final class OtelUtils { | ||
|
||
public static String getComaSeparatedString(Map<String, String> keyValuePairs) { |
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.
public static String getComaSeparatedString(Map<String, String> keyValuePairs) { | |
public static String getCommaSeparatedString(Map<String, String> keyValuePairs) { |
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.
Fixed
Thanks @anuraaga , I am learning the SDK Auto Configure Extension APIs and already have done improvements adopting it on the Jenkins OpenTelemetry Plugin (I use I see this as an iterative journey with enhancement proposals on the SDK Auto Configure Extension APIs and refactoring of the usage of the extension in the Maven and Jenkins integrations. |
c271e74
to
e387b8d
Compare
Yeah ready to merge this after fixing the build in #114 |
…ontrib into use-auto-configure
Description:
Use the Otel SDK Auto Configuration Extension instead of reinventing all the configuration boilerplate.
ℹ️ Note that this PR doesn't extend the supported exporter (e.g. doesn't add support for OTLP HTTP...), we only have OTLP GRPC for the moment, it should be easy to extend as all the logic is provided by the Otel SDK Auto Configuration Extension now.
Existing Issue(s):
Testing:
We don't have auto integration tests yet so tests have to be done manually.
Documentation:
No documentation added because the user experience didn't change.
Outstanding items:
Both items below are tracked in the code as
TODO
.io.opentelemetry.sdk.autoconfigure.spi.ConfigProperties
to pass configuration parameters instead of shoving them in system properties.