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

Allow selecting multiple exporters #1758

Merged
merged 6 commits into from
Jun 23, 2021

Conversation

pellared
Copy link
Member

@pellared pellared commented Jun 11, 2021

Fixes #1755

Changes

  • Allow selecting multiple exporters via OTEL_TRACES_EXPORTER and OTEL_METRICS_EXPORTER by using a comma-separated list.

The exporter selection environmental variables allow only to select a SINGLE one. It would be handy to allow selecting more than one e.g. otlp,console.

@pellared pellared marked this pull request as ready for review June 11, 2021 12:12
@pellared pellared requested review from a team June 11, 2021 12:12
@carlosalberto
Copy link
Contributor

Maybe totally straightforward, but I'd like to see a prototype of this in any language.

@SergeyKanzhelev
Copy link
Member

I think the scenario in PR description otlp,console totally makes a good use cases. In production cases there will be situations when different exporters will need different processors - like a sampling or different aggregations. I suggest we stop at multiple exporters for now in environment variables.

@carlosalberto has a good suggestion. I think it will be trivial to implement in .NET, can you please demonstrate the implementation - issue open-telemetry/opentelemetry-dotnet#1453?

@pellared
Copy link
Member Author

pellared commented Jun 13, 2021

@carlosalberto @SergeyKanzhelev

Maybe totally straightforward, but I'd like to see a prototype of this in any language.

I think it will be trivial to implement in .NET, can you please demonstrate the implementation - issue open-telemetry/opentelemetry-dotnet#1453?

I do not think it is trivial. Especially in .NET as it has almost no support for env vars at the moment. References:

As a rule of thumb, I feel that the design should be similar to OTEL_PROPAGATORS. I made a quick look at the Python SDK. The OTEL_PROPAGATORS implementation looks nice and probably a similar pattern could be used for exporters. WDYT @owais?

I have no idea what is the ETA for implementing open-telemetry/opentelemetry-dotnet#1453. However, I see no problem having this PR open until someone actually has time to implement it (in whatever language) 😉

@jkwatson
Copy link
Contributor

For what it's worth, we implemented this for trace exporters in Java before the env var spec got finalized, at which point we pulled it back to a single exporter. It's a little tricky, however, since just specifying an exporter doesn't determine what span processor you might want to use. And, for a logging exporter, a batch span processor probably doesn't make much sense.

In the otel java instrumentation agent, we automatically add the logging exporter if the user enables debug mode for the javaagent. And, in that case, we wire it up with a SimpleSpanProcessor, as that makes more sense for the logging use-case.

@owais
Copy link
Contributor

owais commented Jun 14, 2021

Co-authored-by: Tigran Najaryan <4194920+tigrannajaryan@users.noreply.github.com>
@SergeyKanzhelev
Copy link
Member

@owais what's the behavior of exporters w.r.t. @jkwatson's comment? Will console exporter will be added with the batch processor? Is there anything worth noting in this PR?

@owais
Copy link
Contributor

owais commented Jun 15, 2021

@SergeyKanzhelev @jkwatson Python implement does not treat console or any other exporter as a special case. It always uses batch exporter. Ability to quickly add console exporter without removing a real exporter is already immensely useful even if the output is a bit delayed because of batching. I have not seen this cause any issues in practice. In fact it actually is quite nice since you can tell exactly when a remote exporter is supposed to export the spans.

Besides, console exporter is not the only use case as this can be used to export to multiple remote exporters as well.

IMO trying to specify multiple combinations of process+exporter via env vars will get too complex and we should expect users to setup such pipelines with code instead.

@carlosalberto
Copy link
Contributor

From the Spec call yesterday: although supporting multiple tracer exporters is recurrent request, supporting multiple metrics exporters is not a clear thing (e.g. pull vs push based systems).

Moreover, this is closely related to the effort by the new Instrumentation group/SIG so let's hold this a little bit for now, while we wait for such group to review it.

@pellared
Copy link
Member Author

pellared commented Jun 16, 2021

@carlosalberto

From the Spec call yesterday: although supporting multiple tracer exporters is recurrent request, supporting multiple metrics exporters is not a clear thing (e.g. pull vs push based systems).

Is there any problem to make them working "side-by-side"? I think that each concrete exporter should have its own set of environmental variables to configure it.

@jkwatson
Copy link
Contributor

@carlosalberto

From the Spec call yesterday: although supporting multiple tracer exporters is recurrent request, supporting multiple metrics exporters is not a clear thing (e.g. pull vs push based systems).

Is there any problem to make them working "side-by-side"? I think that each concrete exporter should have its own set of environmental variables to configure it.

It might not be a problem eventually, but it's certainly not something that is spec'd yet for the metrics SDK, so we shouldn't be providing any configuration for it, IMO.

@SergeyKanzhelev
Copy link
Member

@carlosalberto suggested to wait for instrumentation group to pick this PR up. Thinking about it, this is a small adjustment that doesn't solve problems like different processors and things like this. Just a convenience improvement. So I'd vote to just accept this proposal. Unlikely it will block any sophisticated solution on telemetry pipelines configuration that instrumentation group will come up with. Thoughts?

@jmacd
Copy link
Contributor

jmacd commented Jun 23, 2021

@SergeyKanzhelev and @carlosalberto I'm not sure how this belongs with an instrumentation group. Isn't this about SDK configuration? I would assume that many defaults will apply if I set OTLP_METRICS_EXPORTER=otlp,prometheus and that essential configuration items might get dedicated environment variables (e.g., OTLP_METRICS_PROMETHEUS_PORT, etc.)

@SergeyKanzhelev
Copy link
Member

I don't have any more concerns and @carlosalberto didn't repond for a while, I assume it's ok. Merging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Selecting multiple exporters via environmental variables
9 participants