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

Fix the default endpoint for OTLP exporter when using http/protobuf protocol #2868

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Compat issues with assembly OpenTelemetry.Exporter.OpenTelemetryProtocol:
CannotRemoveAttribute : Attribute 'System.Runtime.CompilerServices.CompilerGeneratedAttribute' exists on 'OpenTelemetry.Exporter.OtlpExporterOptions.Endpoint.get()' in the contract but not the implementation.
CannotRemoveAttribute : Attribute 'System.Runtime.CompilerServices.CompilerGeneratedAttribute' exists on 'OpenTelemetry.Exporter.OtlpExporterOptions.Endpoint.set(System.Uri)' in the contract but not the implementation.
Total Issues: 2
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@
## Unreleased

* LogExporter bug fix to handle null EventName.
([#2870](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2871))
([#2871](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2871))

* Fixed the default endpoint for OTLP exporter over HTTP/Protobuf.
Kielek marked this conversation as resolved.
Show resolved Hide resolved
The default value is `http://localhost:4318`.
([#2868](https://github.com/open-telemetry/opentelemetry-dotnet/pull/2868))

## 1.2.0-rc2

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,20 @@ public class OtlpExporterOptions

internal readonly Func<HttpClient> DefaultHttpClientFactory;

private const string DefaultGrpcEndpoint = "http://localhost:4317";
private const string DefaultHttpEndpoint = "http://localhost:4318";
private const OtlpExportProtocol DefaultOtlpExportProtocol = OtlpExportProtocol.Grpc;

private Uri endpoint;

/// <summary>
/// Initializes a new instance of the <see cref="OtlpExporterOptions"/> class.
/// </summary>
public OtlpExporterOptions()
{
if (EnvironmentVariableHelper.LoadUri(EndpointEnvVarName, out Uri endpoint))
if (EnvironmentVariableHelper.LoadUri(EndpointEnvVarName, out Uri parsedEndpoint))
{
this.Endpoint = endpoint;
this.endpoint = parsedEndpoint;
}

if (EnvironmentVariableHelper.LoadString(HeadersEnvVarName, out string headersEnvVar))
Expand Down Expand Up @@ -79,7 +85,7 @@ public OtlpExporterOptions()

this.HttpClientFactory = this.DefaultHttpClientFactory = () =>
{
return new HttpClient()
return new HttpClient
{
Timeout = TimeSpan.FromMilliseconds(this.TimeoutMilliseconds),
};
Expand All @@ -89,9 +95,30 @@ public OtlpExporterOptions()
/// <summary>
/// Gets or sets the target to which the exporter is going to send telemetry.
/// Must be a valid Uri with scheme (http or https) and host, and
/// may contain a port and path. The default value is http://localhost:4317.
/// may contain a port and path. The default value is
/// * http://localhost:4317 for <see cref="OtlpExportProtocol.Grpc"/>
/// * http://localhost:4318 for <see cref="OtlpExportProtocol.HttpProtobuf"/>.
pellared marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
public Uri Endpoint { get; set; } = new Uri("http://localhost:4317");
public Uri Endpoint
{
get
{
if (this.endpoint == null)
{
this.endpoint = this.Protocol == OtlpExportProtocol.Grpc
? new Uri(DefaultGrpcEndpoint)
: new Uri(DefaultHttpEndpoint);
}

return this.endpoint;
}

set
{
this.endpoint = value;
this.ProgrammaticallyModifiedEndpoint = true;
}
}

/// <summary>
/// Gets or sets optional headers for the connection. Refer to the <a href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/protocol/exporter.md#specifying-headers-via-environment-variables">
Expand All @@ -107,7 +134,7 @@ public OtlpExporterOptions()
/// <summary>
/// Gets or sets the the OTLP transport protocol. Supported values: Grpc and HttpProtobuf.
/// </summary>
public OtlpExportProtocol Protocol { get; set; } = OtlpExportProtocol.Grpc;
public OtlpExportProtocol Protocol { get; set; } = DefaultOtlpExportProtocol;

/// <summary>
/// Gets or sets the export processor type to be used with the OpenTelemetry Protocol Exporter. The default value is <see cref="ExportProcessorType.Batch"/>.
Expand Down Expand Up @@ -167,5 +194,10 @@ public OtlpExporterOptions()
/// </list>
/// </remarks>
public Func<HttpClient> HttpClientFactory { get; set; }

/// <summary>
/// Gets a value indicating whether <see cref="Endpoint" /> was modified via its setter.
/// </summary>
internal bool ProgrammaticallyModifiedEndpoint { get; private set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -155,12 +155,12 @@ public static void TryEnableIHttpClientFactoryIntegration(this OtlpExporterOptio
}
}

internal static void AppendExportPath(this OtlpExporterOptions options, Uri initialEndpoint, string exportRelativePath)
internal static void AppendExportPath(this OtlpExporterOptions options, string exportRelativePath)
{
// The exportRelativePath is only appended when the options.Endpoint property wasn't set by the user,
// the protocol is HttpProtobuf and the OTEL_EXPORTER_OTLP_ENDPOINT environment variable
// is present. If the user provides a custom value for options.Endpoint that value is taken as is.
if (ReferenceEquals(initialEndpoint, options.Endpoint))
if (!options.ProgrammaticallyModifiedEndpoint)
{
if (options.Protocol == OtlpExportProtocol.HttpProtobuf)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,11 @@ private static MeterProviderBuilder AddOtlpExporter(
Action<OtlpExporterOptions> configure,
IServiceProvider serviceProvider)
{
var initialEndpoint = options.Endpoint;

configure?.Invoke(options);

options.TryEnableIHttpClientFactoryIntegration(serviceProvider, "OtlpMetricExporter");

options.AppendExportPath(initialEndpoint, OtlpExporterOptions.MetricsExportPath);
options.AppendExportPath(OtlpExporterOptions.MetricsExportPath);

var metricExporter = new OtlpMetricExporter(options);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,11 @@ private static TracerProviderBuilder AddOtlpExporter(
Action<OtlpExporterOptions> configure,
IServiceProvider serviceProvider)
{
var originalEndpoint = exporterOptions.Endpoint;

configure?.Invoke(exporterOptions);

exporterOptions.TryEnableIHttpClientFactoryIntegration(serviceProvider, "OtlpTraceExporter");

exporterOptions.AppendExportPath(originalEndpoint, OtlpExporterOptions.TracesExportPath);
exporterOptions.AppendExportPath(OtlpExporterOptions.TracesExportPath);

var otlpExporter = new OtlpTraceExporter(exporterOptions);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,9 +173,9 @@ public void AppendExportPath_EndpointNotSet_EnvironmentVariableNotDefined_NotApp

var options = new OtlpExporterOptions { Protocol = OtlpExportProtocol.HttpProtobuf };

options.AppendExportPath(options.Endpoint, "test/path");
options.AppendExportPath("test/path");

Assert.Equal("http://localhost:4317/", options.Endpoint.AbsoluteUri);
Assert.Equal("http://localhost:4318/", options.Endpoint.AbsoluteUri);
}

[Fact]
Expand All @@ -185,7 +185,7 @@ public void AppendExportPath_EndpointNotSet_EnvironmentVariableDefined_Appended(

var options = new OtlpExporterOptions { Protocol = OtlpExportProtocol.HttpProtobuf };

options.AppendExportPath(options.Endpoint, "test/path");
options.AppendExportPath("test/path");

Assert.Equal("http://test:8888/test/path", options.Endpoint.AbsoluteUri);

Expand All @@ -198,10 +198,9 @@ public void AppendExportPath_EndpointSetEqualToEnvironmentVariable_EnvironmentVa
Environment.SetEnvironmentVariable(OtlpExporterOptions.EndpointEnvVarName, "http://test:8888");

var options = new OtlpExporterOptions { Protocol = OtlpExportProtocol.HttpProtobuf };
var originalEndpoint = options.Endpoint;
options.Endpoint = new Uri("http://test:8888");

options.AppendExportPath(originalEndpoint, "test/path");
options.AppendExportPath("test/path");

Assert.Equal("http://test:8888/", options.Endpoint.AbsoluteUri);

Expand All @@ -219,7 +218,7 @@ public void AppendExportPath_EndpointSet_EnvironmentVariableNotDefined_NotAppend
var originalEndpoint = options.Endpoint;
options.Endpoint = new Uri(endpoint);

options.AppendExportPath(originalEndpoint, "test/path");
options.AppendExportPath("test/path");

Assert.Equal(endpoint, options.Endpoint.AbsoluteUri);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,19 @@ public void OtlpExporterOptions_Defaults()
Assert.Equal(OtlpExportProtocol.Grpc, options.Protocol);
}

[Fact]
public void OtlpExporterOptions_DefaultsForHttpProtobuf()
{
var options = new OtlpExporterOptions
{
Protocol = OtlpExportProtocol.HttpProtobuf,
};
Assert.Equal(new Uri("http://localhost:4318"), options.Endpoint);
Assert.Null(options.Headers);
Assert.Equal(10000, options.TimeoutMilliseconds);
Assert.Equal(OtlpExportProtocol.HttpProtobuf, options.Protocol);
}

[Fact]
public void OtlpExporterOptions_EnvironmentVariableOverride()
{
Expand Down Expand Up @@ -104,6 +117,26 @@ public void OtlpExporterOptions_SetterOverridesEnvironmentVariable()
Assert.Equal(OtlpExportProtocol.HttpProtobuf, options.Protocol);
}

[Fact]
public void OtlpExporterOptions_ProtocolSetterDoesNotOverrideCustomEndpointFromEnvVariables()
{
Environment.SetEnvironmentVariable(OtlpExporterOptions.EndpointEnvVarName, "http://test:8888");

var options = new OtlpExporterOptions { Protocol = OtlpExportProtocol.Grpc };

Assert.Equal(new Uri("http://test:8888"), options.Endpoint);
Assert.Equal(OtlpExportProtocol.Grpc, options.Protocol);
}

[Fact]
public void OtlpExporterOptions_ProtocolSetterDoesNotOverrideCustomEndpointFromSetter()
{
var options = new OtlpExporterOptions { Endpoint = new Uri("http://test:8888"), Protocol = OtlpExportProtocol.Grpc };

Assert.Equal(new Uri("http://test:8888"), options.Endpoint);
Assert.Equal(OtlpExportProtocol.Grpc, options.Protocol);
}

[Fact]
public void OtlpExporterOptions_EnvironmentVariableNames()
{
Expand Down