-
-
Notifications
You must be signed in to change notification settings - Fork 456
POTEL 64 - sentry-opentelemetry-agentless
module
#3961
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
sentry-opentelemetry-agentless
modulesentry-opentelemetry-agentless
module
|
Performance metrics 🚀
|
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.
Left some comments regarding auto configuration that we need to figure out
api(Config.Libs.OpenTelemetry.otelSdk) | ||
api(Config.Libs.OpenTelemetry.otelSemconv) | ||
api(Config.Libs.OpenTelemetry.otelSemconvIncubating) | ||
implementation(Config.Libs.OpenTelemetry.otelExtensionAutoconfigure) |
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.
Needs to be declared with api
scope so that any depending Applications can use the AutoConfiguredOpenTelemetrySdk
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.
We need to initialize OpenTelemetry by using AutoConfiguredOpenTelemetrySdk
as explained here: https://opentelemetry.io/docs/languages/java/configuration/#zero-code-sdk-autoconfigure.
The implicit GlobalOpentelemetry.get()
does not do automatic configuration which we need for our spis to load.
Also, just using AutoConfiguredOpenTelemetrySdk.initialize()
as shown in the documentation above ist not enough because by default it tries to use oltp
as exporter for which we do not have dependencies added and therefore crashes.
The following piece of code works for initializing the sdk:
AutoConfiguredOpenTelemetrySdk.builder()
.setResultAsGlobal()
.addPropertiesSupplier(
() -> {
final Map<String, String> properties = new HashMap<>();
properties.put("otel.logs.exporter", "none");
properties.put("otel.metrics.exporter", "none");
properties.put("otel.traces.exporter", "none");
return properties;
})
.build();
AutoConfiguredOpenTelemetrySdk.initialize()
would work only in conjunction with environment variables setting the exporters to none.
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.
The application can also be started using -Dotel.java.global-autoconfigure.enabled=true
which means opentelemetry-sdk-extension-autoconfigure
will take care of this.
Customers also have to add OTEL_LOGS_EXPORTER=none;OTEL_METRICS_EXPORTER=none;OTEL_TRACES_EXPORTER=none
as env vars, to avoid spammy logs.
We can offer both ways in docs.
For more details on configuring Sentry via `sentry.properties` please see the | ||
[docs page](https://docs.sentry.io/platforms/java/configuration/). | ||
|
||
As an alternative to the `SENTRY_PROPERTIES_FILE` environment variable you can provide individual |
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.
We'll need to add some info on how to initialize with AutoConfiguredOpenTelemetrySdk
. Or find another way to do that in e.g. OtelSpanFactory.getTracerProvider()
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.
Let's follow up in a different PR.
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.
LGTM 👍
📜 Description
Add
sentry-opentelemetry-agentless
module that customers can add as a single dependency to use our OpenTelemetry offering instead of having to add multiple dependencies and manage versions.💡 Motivation and Context
Closes #3913
💚 How did you test it?
📝 Checklist
sendDefaultPII
is enabled.🔮 Next steps
agentless-spring-boot
that pulls inopentelemetry-spring-boot-starter
with the required version