-
Notifications
You must be signed in to change notification settings - Fork 857
[otlp] Remove the Google.Protobuf / Grpc packages, and replace the logs and metrics with the new implementation #6005
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
Changes from all commits
1f551af
9aa77bc
cb52d26
d7b7797
8a0beb9
0599950
bf4becd
d07f706
41dc362
c54a555
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,14 +9,14 @@ | |
|
|
||
| namespace OpenTelemetry.Exporter.OpenTelemetryProtocol.Implementation.ExportClient; | ||
|
|
||
| internal abstract class ProtobufOtlpExportClient : IProtobufExportClient | ||
| internal abstract class OtlpExportClient : IExportClient | ||
| { | ||
| private static readonly Version Http2RequestVersion = new(2, 0); | ||
|
|
||
| #if NET | ||
| private static readonly bool SynchronousSendSupportedByCurrentPlatform; | ||
|
|
||
| static ProtobufOtlpExportClient() | ||
| static OtlpExportClient() | ||
| { | ||
| #if NET | ||
| // See: https://github.com/dotnet/runtime/blob/280f2a0c60ce0378b8db49adc0eecc463d00fe5d/src/libraries/System.Net.Http/src/System/Net/Http/HttpClientHandler.AnyMobile.cs#L767 | ||
|
|
@@ -28,13 +28,24 @@ static ProtobufOtlpExportClient() | |
| } | ||
| #endif | ||
|
|
||
| protected ProtobufOtlpExportClient(OtlpExporterOptions options, HttpClient httpClient, string signalPath) | ||
| protected OtlpExportClient(OtlpExporterOptions options, HttpClient httpClient, string signalPath) | ||
| { | ||
| Guard.ThrowIfNull(options); | ||
| Guard.ThrowIfNull(httpClient); | ||
| Guard.ThrowIfNull(signalPath); | ||
|
|
||
| Uri exporterEndpoint = options.Endpoint.AppendPathIfNotPresent(signalPath); | ||
| Uri exporterEndpoint; | ||
| if (options.Protocol == OtlpExportProtocol.Grpc) | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a logical condition here to honor the existing behavior.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Curious how it got by tests before?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New implementation was never tested on it. On the replacement we caught this issue.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The actual issue was with the HTTP implementation. In the gRPC case, we append the path in all scenarios. However, for HTTP, the path is appended only if it is provided by default or through environment variables. If an endpoint is explicitly set, we do not append the path in the HTTP case, but this behavior was not being followed in new implementation. During my initial implementation, I mistakenly assumed this was a bug and removed the check. However, there was an existing test case that validated this behavior. |
||
| { | ||
| exporterEndpoint = options.Endpoint.AppendPathIfNotPresent(signalPath); | ||
| } | ||
| else | ||
| { | ||
| exporterEndpoint = options.AppendSignalPathToEndpoint | ||
| ? options.Endpoint.AppendPathIfNotPresent(signalPath) | ||
| : options.Endpoint; | ||
| } | ||
|
|
||
| this.Endpoint = new UriBuilder(exporterEndpoint).Uri; | ||
| this.Headers = options.GetHeaders<Dictionary<string, string>>((d, k, v) => d.Add(k, v)); | ||
| this.HttpClient = httpClient; | ||
|
|
||
This file was deleted.
Uh oh!
There was an error while loading. Please reload this page.