Update OpenTelemetry Protocol Exporter #643
Conversation
src/OpenTelemetry.Exporter.OpenTelemetryProtocol/Implementation/SpanDataExtensions.cs
Show resolved
Hide resolved
...nTelemetryProtocol/Implementation/opentelemetry/proto/collector/trace/v1/trace_service.proto
Show resolved
Hide resolved
| { | ||
| using var tracerFactory = TracerFactory.Create(builder => builder | ||
| .SetResource(Resources.CreateServiceResource("otlp-test")) | ||
| .UseOpenTelemetryProtocol(config => config.Endpoint = endpoint)); |
There was a problem hiding this comment.
this is just a point for discussion, not blocking. THis name UseOpenTelemetryProtocol does not sound descriptive enough. Not clear that this is an exporter
There was a problem hiding this comment.
What do you think about naming it UseOtlpExporter? OTLP is how this is commonly known.
There was a problem hiding this comment.
C# naming guidelines recommend to not abbreviate when possible. Would UseOpenTelemetryProtocolExporter work better? Or you think it's too long?
There was a problem hiding this comment.
It is a bit on the long side but typically it will be used only once per app, will update the PR soon.
...nTelemetry.Exporter.OpenTelemetryProtocol/Implementation/opentelemetry/proto/agent/README.md
Show resolved
Hide resolved
| private static OtlpTrace.Status ToOtlpStatus(Status status) | ||
| { | ||
| // At this stage Status.IsValid is always true, just add status message if !Ok | ||
| if (status == Status.Ok) |
There was a problem hiding this comment.
OK may have a description. Why not add it?
There was a problem hiding this comment.
This is comparing the whole struct so if a description was set it will be included. I will add a test to ensure that the behavior is not broken in the future.
There was a problem hiding this comment.
Actually, I'm not finding a way to set the description. Opened issue #653.
|
|
||
| foreach (var resource in resourceToLibraryAndSpans) | ||
| { | ||
| // TODO: this is a temporary workaround since library is still on the resource, not on its own field. |
There was a problem hiding this comment.
Follow-up on issue #649
| { | ||
| otlpSpan.Attributes.AddRange(spanData.Attributes.Select(ToOtlpAttribute)); | ||
|
|
||
| // TODO: get dropped count. |
| ["string"] = "text", | ||
| ["double"] = 3.14, | ||
| ["unknow_attrib_type"] = | ||
| new byte[] { 1 }, // TODO: update when arrays of standard attribute types are supported |
8333423 to
23f583e
Compare
Update the OTLP to version 0.3.0 of the proto files.
7679801 to
4ddfb99
Compare
Fix Issue #19.
Changes
Update the OTLP to version 0.3.0 of the proto files.
This does not address the issue of how to better handle the proto files, this will be done separately.
Checklist
Notice: that there were breaks on projects unrelated to the changes (tests on were run via
dotnet test OpenTelemetry.slnon a mac.