Skip to content
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

Autoconfigurable Telemetry SDK support #9453

Conversation

Shephalimittal
Copy link
Contributor

@Shephalimittal Shephalimittal commented Aug 21, 2023

Description

Added support for OpenTelemetry SDK Autoconfigure.
This artifact implements environment-based autoconfiguration of the OpenTelemetry SDK. This can be an alternative to programmatic configuration using the normal SDK builders.

All options support being passed as Java system properties, e.g., -Dotel.traces.exporter=zipkin or environment variables, e.g., OTEL_TRACES_EXPORTER=zipkin. For more details refer here.

The intention here is to remove managing tracing configuration like exporter, endpoints, processor delays, sampler etc through Opensearch cluster settings. Also, if there are any custom exporter or sampler settings, user can pass that through jvm options or environment variables.

This also provides support to add custom exporters. InMemorySpanExporter is the exporter used for integration tests, have made it autoconfigurable by using ConfigurableSpanExporterProvider class

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff
  • Commit changes are listed out in CHANGELOG.md file (See: Changelog)

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@Shephalimittal
Copy link
Contributor Author

@reta Please review.

@reta
Copy link
Collaborator

reta commented Aug 23, 2023

@reta Please review.

@Shephalimittal please, first of all, fill the description for this change, so everyone would understand what is the purpose of it. Please also add the changelog entry under [Unreleased 2.x] section, thank you.

@reta
Copy link
Collaborator

reta commented Aug 23, 2023

@Shephalimittal OTel auto configuration is certainly useful for application developers, now I am struggling to understand how that will work in OpenSeach:

  • user installs OpenSeach
  • user install OpenSearch Otel plugin

User wants to configure OpenSearch Otel plugin to send traces to Jaeger (for example), how that could be accomplished with auto configuration?

@Gaganjuneja
Copy link
Contributor

@Shephalimittal OTel auto configuration is certainly useful for application developers, now I am struggling to understand how that will work in OpenSeach:

  • user installs OpenSeach
  • user install OpenSearch Otel plugin

User wants to configure OpenSearch Otel plugin to send traces to Jaeger (for example), how that could be accomplished with auto configuration?

As per my understanding, user will have to provide the jaeger related configurations in the jvm.options file.

@reta
Copy link
Collaborator

reta commented Aug 23, 2023

As per my understanding, user will have to provide the jaeger related configurations in the jvm.options file.

If that is the case (which could only be done for some properties), this is certainly no go: we have established settings management.

@Gaganjuneja
Copy link
Contributor

As per my understanding, user will have to provide the jaeger related configurations in the jvm.options file.

If that is the case (which could only be done for some properties), this is certainly no go: we have established settings management.

Only challenge with doing setting management ourselves is we always need to sync with the OTEL. The OTLPGrpcSpanExporter itself has more than 10 settings and similarly BatchSpanProcessor. So we need to expose these all through OpenSearch settings and if there are new types of exporters we might have to add support.

OTelAutoConfigureSDK reads the settings/configuration from System properties so not sure if opensearch.yml can also be used for the same.

@Shephalimittal Shephalimittal requested a review from msfroh as a code owner August 25, 2023 19:27
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@Shephalimittal
Copy link
Contributor Author

@reta Please review.

@reta
Copy link
Collaborator

reta commented Aug 28, 2023

@reta Please review.

@Shephalimittal please answer my questions (#9453 (comment))

@Shephalimittal
Copy link
Contributor Author

@Shephalimittal OTel auto configuration is certainly useful for application developers, now I am struggling to understand how that will work in OpenSeach:

  • user installs OpenSeach
  • user install OpenSearch Otel plugin

User wants to configure OpenSearch Otel plugin to send traces to Jaeger (for example), how that could be accomplished with auto configuration?

Yes, that will be achieved through this setting -Dotel.traces.exporter=jaeger in jvm.options. For details refer link

Also, if user wants to set up a custom exporter, that can be provided using implementing ConfigurableSpanExporterProvider. I have added InMemorySingletonSpanExporterProvider for integration test.

@reta
Copy link
Collaborator

reta commented Aug 28, 2023

Thanks @Shephalimittal

Yes, that will be achieved through this setting -Dotel.traces.exporter=jaeger in jvm.options. For details refer link

We use jvm.options to configure the JVM parameters for JVM process, not plugins / module or extensions: we use opensearch.yaml for configuration of deployment specific setting and this is what the otel-plugin should also do.

Also, if user wants to set up a custom exporter, that can be provided using implementing ConfigurableSpanExporterProvider. I have added InMemorySingletonSpanExporterProvider for integration test.

Very well, but in most cases the additional dependency (primarily, exporters) have to be provided (and we cannot bundle everything), so it brings us to the the following question: how user should do that? Manually copying JAR files does not look like a viable option at all (since those may require additional dependencies).

@Shephalimittal
Copy link
Contributor Author

Only challenge with doing setting management ourselves is we always need to sync with the OTEL. The OTLPGrpcSpanExporter itself has more than 10 settings and similarly BatchSpanProcessor. So we need to expose these all through OpenSearch settings and if there are new types of exporters we might have to add support.

Thanks @Shephalimittal

Yes, that will be achieved through this setting -Dotel.traces.exporter=jaeger in jvm.options. For details refer link

We use jvm.options to configure the JVM parameters for JVM process, not plugins / module or extensions: we use opensearch.yaml for configuration of deployment specific setting and this is what the otel-plugin should also do.
But problem with using cluster settings is : (Pasting Gagan's point from above)
Only challenge with doing setting management ourselves is we always need to sync with the OTEL. The OTLPGrpcSpanExporter itself has more than 10 settings and similarly BatchSpanProcessor. So we need to expose these all through OpenSearch settings and if there are new types of exporters we might have to add support.

Also, if user wants to set up a custom exporter, that can be provided using implementing ConfigurableSpanExporterProvider. I have added InMemorySingletonSpanExporterProvider for integration test.

Very well, but in most cases the additional dependency (primarily, exporters) have to be provided (and we cannot bundle everything), so it brings us to the the following question: how user should do that? Manually copying JAR files does not look like a viable option at all (since those may require additional dependencies).

@Shephalimittal
Copy link
Contributor Author

Only challenge with doing setting management ourselves is we always need to sync with the OTEL. The OTLPGrpcSpanExporter itself has more than 10 settings and similarly BatchSpanProcessor. So we need to expose these all through OpenSearch settings and if there are new types of exporters we might have to add support.

Thanks @Shephalimittal

Yes, that will be achieved through this setting -Dotel.traces.exporter=jaeger in jvm.options. For details refer link

We use jvm.options to configure the JVM parameters for JVM process, not plugins / module or extensions: we use opensearch.yaml for configuration of deployment specific setting and this is what the otel-plugin should also do.
But problem with using cluster settings is : (Pasting Gagan's point from above)
Only challenge with doing setting management ourselves is we always need to sync with the OTEL. The OTLPGrpcSpanExporter itself has more than 10 settings and similarly BatchSpanProcessor. So we need to expose these all through OpenSearch settings and if there are new types of exporters we might have to add support.

Also, if user wants to set up a custom exporter, that can be provided using implementing ConfigurableSpanExporterProvider. I have added InMemorySingletonSpanExporterProvider for integration test.

Very well, but in most cases the additional dependency (primarily, exporters) have to be provided (and we cannot bundle everything), so it brings us to the the following question: how user should do that? Manually copying JAR files does not look like a viable option at all (since those may require additional dependencies).

Exploring the option to use AffixSettings here for telemetry settings regarding exporters, delays, timeouts, samplers etc and passing these settings as configProperties to AutoConfiguredOpenTelemetrySdk. @reta Any Thoughts?

@reta
Copy link
Collaborator

reta commented Aug 29, 2023

Exploring the option to use AffixSettings here for telemetry settings regarding exporters, delays, timeouts, samplers etc and passing these settings as configProperties to AutoConfiguredOpenTelemetrySdk. @reta Any Thoughts?

@Shephalimittal we need more than that:

  • dependencies (new exporters and samplers)
  • security policy changes

In general, I think we have to take a step back and open RFC issue to outline the vision regarding extensibiltity / customization of the out OTEL plugin (I personally don't see the large picture).

@opensearch-trigger-bot
Copy link
Contributor

This PR is stalled because it has been open for 30 days with no activity.

@opensearch-trigger-bot opensearch-trigger-bot bot added the stalled Issues that have stalled label Sep 29, 2023
@ticheng-aws
Copy link
Contributor

Hi @Shephalimittal, the PR is stalled. Is this being worked upon? Feel free to reach out to maintainers for further reviews.

@ticheng-aws ticheng-aws added the enhancement Enhancement or improvement to existing feature or request label Jan 6, 2024
@opensearch-trigger-bot opensearch-trigger-bot bot removed the stalled Issues that have stalled label Jan 10, 2024
@opensearch-trigger-bot
Copy link
Contributor

This PR is stalled because it has been open for 30 days with no activity.

@opensearch-trigger-bot opensearch-trigger-bot bot added the stalled Issues that have stalled label Feb 17, 2024
@kotwanikunal
Copy link
Member

@Shephalimittal Closing this stalled PR. Please reopen to continue with the contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement or improvement to existing feature or request stalled Issues that have stalled
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants