- Notifications
You must be signed in to change notification settings - Fork1.2k
Initial OTELp support#20585
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Initial OTELp support#20585
Uh oh!
There was an error while loading.Please reload this page.
Conversation
When I looked at this, it looked tricky to do, as you'll need to collect traces from sub-processes as well. I didn't see the package you're linking though, so maybe it's possible. The best I had come up with is to maybe run a collector in the main CLI and have all plugins connect back to that to send traces.
Jaeger does have a "export to JSON" option, which might be workable. It's obviously not as nice as just writing to a file directly, but might be okay if it makes things easier.
We might want to support both for a while, since users might be using older providers which only support opentracing. It's also a breaking change to remove |
We might still need to do that. I think this package is more about a stable file format. We will still need to figure out how to pass a server address down to language hosts (and ultimately providers).
I saw that. I'm really hoping it won't be necessary, but let's see. 😄
I'm of 2 minds here. Removing the We could start by adding an |
Yeah I think that's the way to go. Imo we should just keep |
The issue there is how traces are emitted from our CLI code. If I want to add a new span to a function and I use OTEL to do so, then it won't show up in the opentracing output. Keeping |
Sure that's fine, we don't need anynew stuff to show up in |
Uh oh!
There was an error while loading.Please reload this page.
Switch the CLI over to using
OTEL
.TODO
--tracing
implementation, it needs a changelog entry.