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

Support OpenTelemetry protocol and agent exporter #19

Closed
SergeyKanzhelev opened this issue May 10, 2019 · 15 comments
Closed

Support OpenTelemetry protocol and agent exporter #19

SergeyKanzhelev opened this issue May 10, 2019 · 15 comments
Assignees
Labels
pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package

Comments

@SergeyKanzhelev
Copy link
Member

Find protos here: https://github.com/open-telemetry/opentelemetry-java/tree/master/sdk/src/main/proto

@SergeyKanzhelev SergeyKanzhelev added the pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package label May 10, 2019
@trwegner
Copy link
Contributor

Also should update src\OpenTelemetry.Exporter.Ocagent\Implementation\SpanDataExtentions.cs#39 with producer and consumer span types in the new protos

@austinlparker
Copy link
Member

fyi the protos are in https://github.com/open-telemetry/opentelemetry-proto now

@austinlparker
Copy link
Member

One thing I've wondered, would it make sense to publish the generated proto code out of that repo rather than importing it to this one as a submodule or so forth?

@SergeyKanzhelev
Copy link
Member Author

I have very little experience with submodules. Every time I tried it was not a streamline thing. It always took more time to fix tooling than simple copying of rarely updated files take. And shipping a hotfix that only addresses a single issue and only need one of many changes is impossible.

If you have good examples where this approach worked just fine - I'm happy to try again and learn.

@austinlparker
Copy link
Member

I've found submodules to be unwieldy as well - we can just copy files around for now I reckon.

@SergeyKanzhelev SergeyKanzhelev added this to the SDK complete milestone Jun 1, 2019
@SergeyKanzhelev SergeyKanzhelev modified the milestones: SDK complete, Basic exporters and adapters Jun 11, 2019
@lmolkova lmolkova modified the milestones: Basic exporters and adapters, Future Oct 12, 2019
@lmolkova lmolkova changed the title Use new proto files in OpenTelemetry exporter Support OpenTelemetry protocol and agent exporter Oct 12, 2019
@pcwiese
Copy link
Contributor

pcwiese commented Nov 7, 2019

Have feelings changed about how to proceed with these .proto files? At the very least a reference to a commit from the proto repo is necessary. We could copy the .proto files over and generate as part of the build of this project I think. No need to check-in the intermediate g.cs files.

@SergeyKanzhelev
Copy link
Member Author

Java went submodule way: https://github.com/open-telemetry/opentelemetry-java/tree/master/proto/src/main We can try the same

@SergeyKanzhelev
Copy link
Member Author

I don't have strong feelings

@pcwiese
Copy link
Contributor

pcwiese commented Nov 7, 2019

Ugh. submodules sounds nice in principle and a bit nasty in practice. I'll make a copy of the files.

@SergeyKanzhelev
Copy link
Member Author

As discussed on Gitter, @pjanotti assigning this to you

@cijothomas
Copy link
Member

This should be complete now. @pjanotti Can you confirm and close this?

@pcwiese
Copy link
Contributor

pcwiese commented Jul 16, 2020

We are still missing the instrumentation library in the exports...

@reyang reyang removed this from the Future milestone Jul 28, 2020
@cijothomas
Copy link
Member

@eddynaka please check if the instrumentationlibrary info is missing in the otlp exporter.

@cijothomas
Copy link
Member

We are still missing the instrumentation library in the exports...

This is because ActivitySource is blank for those DiagnosticSourceInstrumentation activities. One proposed fix would be to modify the legacy activity with correct ActivitySource.

@cijothomas
Copy link
Member

This work is complete. The lack of InstrumentationLibrary info is a bug in the instrumentations, not an exporter. Closing this.

Yun-Ting referenced this issue in Yun-Ting/opentelemetry-dotnet Oct 13, 2022
…ed .editorconfig from primary repo. Added some missing files in solution. (#19)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:OpenTelemetry.Exporter.OpenTelemetryProtocol Issues related to OpenTelemetry.Exporter.OpenTelemetryProtocol NuGet package
Projects
None yet
Development

No branches or pull requests

8 participants