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

Make OTel Collector serve Jaeger Remote Sampling directly #500

Closed
ledor473 opened this issue Jan 14, 2020 · 12 comments · Fixed by #547
Closed

Make OTel Collector serve Jaeger Remote Sampling directly #500

ledor473 opened this issue Jan 14, 2020 · 12 comments · Fixed by #547
Labels
help wanted Good issue for contributors to OpenTelemetry Service to pick up

Comments

@ledor473
Copy link
Contributor

The Jaeger Remote Sampling was introduced in #448 but as of it is now, it only fetches the configuration from a remote Jaeger Collector. This makes it impossible to drop the OTel Collector as a replacement for the Jaeger Collector.

If we could add a way to parse and serve the sampling configuration directly, we would be closer to be able to replace it. Here's the related documentation: https://www.jaegertracing.io/docs/1.16/sampling/#collector-sampling-configuration

The architecture without OTel would look like this:

[ Application ] --> [ Jaeger Agent ] --> [ Jaeger Collector ] --> [ Kafka ] <-- [ Jaeger Ingester ]

The current implementation would force us to run this:

[ Application ] --> [ OTel/Jaeger Agent ] --> [ OTel Collector ] --> [ Jaeger Collector ] --> [ Kafka ] <-- [ Jaeger Ingester ]

Ideally, we would run this:

[ Application ] --> [ OTel/Jaeger Agent ] --> [ OTel Collector ] --> [ Kafka ] <-- [ Jaeger Ingester ]
@pjanotti pjanotti added help wanted Good issue for contributors to OpenTelemetry Service to pick up up-for-grabs labels Jan 15, 2020
@joe-elliott
Copy link
Contributor

Does the Opentelemetry collector have a Kafka exporter that would make the ideal option possible?

@ledor473
Copy link
Contributor Author

@joe-elliott I just looked and while OpenCensus Collector had one, it was removed when it changed to OTel Collector.
I guess the idea was to move it to https://github.com/open-telemetry/opentelemetry-collector-contrib, but it's not there neither.

@joe-elliott
Copy link
Contributor

We are currently planning to run the below which has the same number of components as your ideal path and supports remote sampling. Would this work for you all?

[ Application ] --> [ OTel "Agent" ] -->  [ Jaeger Collector ] --> [ Kafka ] --> [ Jaeger Ingester ]

I intend to submit fixes for #491 and #492 this week which should make this a possibility.

@ledor473
Copy link
Contributor Author

It wouldn't work for what we are trying to do. The idea is to experiment with OTel Collector features (tail sampling, zpages, multiples exporters) while keeping the functionalities of Jaeger almost the same (Remote Sampling hardcoded in a file, the UI, etc)

@joe-elliott
Copy link
Contributor

joe-elliott commented Jan 30, 2020

I believe this is really two separate issues:

  • Add a Jaeger Kafka exporter
  • Add support for loading a json file and serving the Jaeger sampling strategy

In both cases this would be replacing pieces of the Jaeger backend instead of just supporting export to the Jaeger backend. As such, if we decide to do it, I think they should be implemented in contrib instead of the main repo.

Kafka
The Kafka exporter is "easy". The Jaeger Kafka storage implements a clean interface: https://github.com/jaegertracing/jaeger/tree/master/plugin/storage/kafka

Sampling Strategies
Serving the sampling strategy gets a little bit more complex. Jaeger implements a clean interface so the code wouldn't be hard: https://github.com/jaegertracing/jaeger/tree/master/plugin/sampling/strategystore/static. It would not be difficult to implement this as an extension in the contrib repo.

However, the difficult part is that the Jaeger Collector accepts spans on the same port that it serves the sampling strategy. Additionally the Jaeger agent can not be configured to push spans to a different port then it gets its sampling strategy from (otelcol as a Jaeger agent can). Options include:

  1. Sampling strategies go in contrib as an extension
    In this case strategies and spans would be on different ports and therefore would be incompatible with the Jaeger agent, but you could use otelcol in its place.

  2. Sampling strategies go in the main repo as part of the Jaeger receiver
    In this case we support the Jaeger agent directly, but perhaps are obscuring the role of the otel collector.

@tigrannajaryan @pjanotti Thoughts?

@ledor473
Copy link
Contributor Author

ledor473 commented Feb 3, 2020

I believe this is really two separate issues
Totally! This current issue is only about Add support for loading a json file and serving the Jaeger sampling strategy

The rest makes sense. Good catch about that same port problem 👍

@pjanotti
Copy link
Contributor

pjanotti commented Feb 3, 2020

@joe-elliott I think option 2 is reasonable: it keeps the same behavior that the Jaeger agent expects and it is part of their "protocol". It should be configurable so it can be enabled/disabled as needed.

@pjanotti
Copy link
Contributor

pjanotti commented Feb 3, 2020

@ledor473 @joe-elliott how common is the usage of the Kafka exporter on Jaeger Collector? At first, my guess is that it fits better on the contrib repo but not 100% sure.

@joe-elliott
Copy link
Contributor

@pjanotti

I think option 2 is reasonable: it keeps the same behavior that the Jaeger agent expects and it is part of their "protocol". It should be configurable so it can be enabled/disabled as needed.

So I don't mind doing this in main repo. It would be easier and more flexible than putting it in contrib.

I was just pointing out that these changes would be the first things that replace Jaeger collector functionality. Everything else we've done interfaces with the Jaeger collector and sees that as the beginning of the "Jaeger backend".

As far as usage of the Kafka exporter, I can't say. We personally use Kafka.

I would be glad to move forward with both of these additions in either contrib or the main repo pending a decision.

@ledor473
Copy link
Contributor Author

ledor473 commented Feb 4, 2020

We do use Kafka as well and as far as I know Uber does as well

@pjanotti
Copy link
Contributor

pjanotti commented Feb 5, 2020

@joe-elliott we don't target replacing the Jaeger collector but my impression is that the usability of the Jaeger agent receiver itself gets very limited by the lack of supporting the config file. That's why my preference is to implement it on the core repo. /cc @open-telemetry/collector-maintainers and @open-telemetry/collector-approvers

@ledor473 we can start with a Kafka exporter on contrib and later if deemed appropriate we can move it to core.

tigrannajaryan pushed a commit that referenced this issue Feb 20, 2020
Added support for hosting a jaeger remote sampling file as requested in #500. This was added to the core repo per discussion there.

Link to tracking Issue:
Fixes #500

Testing:
Tests were added covering config, receiver creation and receiver functionality.

Documentation:
Added documentation on the new strategy_file config option.
@zcross
Copy link

zcross commented Jul 7, 2023

I've been catching up on this (fairly old) issue and the lineage of changes related to JaegerRemoteSampler, such as this mix of merged/abandoned PRs:

One thing that's throwing me off a little:

  • The OTEL spec now defines JaegerRemoteSampler, and a few agent implementations (e.g,. Go, Rust, Java) support it
  • However, the collector code here is packaged in jaegerreceiver.

Is it accurate to interpret this functionality (collector proxying config files to agents) as being usable by either Jaeger instrumentation or supporting OpenTelemetry agent implementations? I.e., can OTEL agents retrieve this config from collector (when configured to hit the config endpoint) even if they're not actually submitting Jaeger-format traces at all? Maybe the packaging is an artifact of some portion of the code being reused from jaeger, and not wanting that to leak out into the core collector packages?

Finally, I find this line of work to be interesting and worth continuing: #2681. That PR has been abandoned for some time because the author didn't have the bandwidth to finish. Is there any other (e.g,. new in the years that have passed) blocker to building such polling behavior, or to further extend this remote sampler configuration serving functionality with optional/configurable additions?

Edit: today, I discovered this seemingly newer (from 2022) "extension" (!= "receiver") for JaegerRemoteSampler configuration serving. Is anyone with an interest in this (i.e., in this issue or other linked issues) interested in and/or using this new extension? Should it be perceived as a preferred alternative to the jaegerreceiver package functionality?

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Good issue for contributors to OpenTelemetry Service to pick up
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants