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

Rely on DefaultResource's Service.Name for Exporters #1768

Merged
merged 20 commits into from
Feb 2, 2021
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
f748caa
Jaeger and API changes and ProviderMethod
Austin-Tan Feb 1, 2021
c606aad
Changelog
Austin-Tan Feb 1, 2021
1908b14
Fix whitespace
Austin-Tan Feb 1, 2021
2dc5b83
Not just checking first element of default resource but querying for …
Austin-Tan Feb 1, 2021
a130385
Including the servicename fallback in Zipkin options
Austin-Tan Feb 1, 2021
9f659f8
Resolving tests
Austin-Tan Feb 1, 2021
9e14115
we previously removed the adapting of new servicenames from resources…
Austin-Tan Feb 1, 2021
65ea610
Removing servicename from Jaeger ctor
Austin-Tan Feb 2, 2021
d3228c5
re-adding servicename to Ctor for Jaeger
Austin-Tan Feb 2, 2021
1c484f9
Merge branch 'main' of https://github.com/open-telemetry/opentelemetr…
Austin-Tan Feb 2, 2021
1dd9529
CodeBlanch 's implementation for preserving servicename
Austin-Tan Feb 2, 2021
4bc9b0d
Scrubbed ServiceName from ZipkinExporterOptions
Austin-Tan Feb 2, 2021
d4e4dcd
Messed up ternary operator order again
Austin-Tan Feb 2, 2021
e6639bd
reordering using for SA compliance
Austin-Tan Feb 2, 2021
5cbabac
more instances of zipkinoptions.servicename
Austin-Tan Feb 2, 2021
20f21c1
removed servicename from API
Austin-Tan Feb 2, 2021
6b7743b
Merge branch 'main' into ExportersGrabDefaultResource
cijothomas Feb 2, 2021
089d216
Changelog in right places
Austin-Tan Feb 2, 2021
4f21dbb
Merge branch 'ExportersGrabDefaultResource' of https://github.com/Aus…
Austin-Tan Feb 2, 2021
88846d0
Merge branch 'main' into ExportersGrabDefaultResource
cijothomas Feb 2, 2021
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
1 change: 0 additions & 1 deletion examples/AspNetCore/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ public void ConfigureServices(IServiceCollection services)
.AddHttpClientInstrumentation()
.AddZipkinExporter(zipkinOptions =>
{
zipkinOptions.ServiceName = this.Configuration.GetValue<string>("Zipkin:ServiceName");
zipkinOptions.Endpoint = new Uri(this.Configuration.GetValue<string>("Zipkin:Endpoint"));
}));
break;
Expand Down
1 change: 0 additions & 1 deletion examples/Console/TestRedis.cs
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ internal static object Run(string zipkinUri)
using var openTelemetry = Sdk.CreateTracerProviderBuilder()
.AddZipkinExporter(o =>
{
o.ServiceName = "redis-test";
o.Endpoint = new Uri(zipkinUri);
})
.AddRedisInstrumentation(connection, options =>
Expand Down
1 change: 0 additions & 1 deletion examples/Console/TestZipkinExporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ internal static object Run(string zipkinUri)
.AddSource("Samples.SampleClient", "Samples.SampleServer")
.AddZipkinExporter(o =>
{
o.ServiceName = "test-zipkin";
o.Endpoint = new Uri(zipkinUri);
})
.Build();
Expand Down
1 change: 0 additions & 1 deletion examples/GrpcService/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,6 @@ public void ConfigureServices(IServiceCollection services)
.AddAspNetCoreInstrumentation()
.AddZipkinExporter(zipkinOptions =>
{
zipkinOptions.ServiceName = this.Configuration.GetValue<string>("Zipkin:ServiceName");
zipkinOptions.Endpoint = new Uri(this.Configuration.GetValue<string>("Zipkin:Endpoint"));
}));
break;
Expand Down
1 change: 0 additions & 1 deletion examples/MicroserviceExample/WebApi/Startup.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ public void ConfigureServices(IServiceCollection services)
.AddZipkinExporter(b =>
{
var zipkinHostName = Environment.GetEnvironmentVariable("ZIPKIN_HOSTNAME") ?? "localhost";
b.ServiceName = nameof(WebApi);
b.Endpoint = new Uri($"http://{zipkinHostName}:9411/api/v2/spans");
}));
}
Expand Down
1 change: 0 additions & 1 deletion examples/MicroserviceExample/WorkerService/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ public static IHostBuilder CreateHostBuilder(string[] args) =>
.AddZipkinExporter(b =>
{
var zipkinHostName = Environment.GetEnvironmentVariable("ZIPKIN_HOSTNAME") ?? "localhost";
b.ServiceName = nameof(WorkerService);
b.Endpoint = new Uri($"http://{zipkinHostName}:9411/api/v2/spans");
});
});
Expand Down
17 changes: 9 additions & 8 deletions src/OpenTelemetry.Exporter.Jaeger/JaegerExporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Runtime.CompilerServices;
using OpenTelemetry.Exporter.Jaeger.Implementation;
using OpenTelemetry.Resources;
Expand All @@ -28,8 +29,6 @@ namespace OpenTelemetry.Exporter
{
public class JaegerExporter : BaseExporter<Activity>
{
private const string DefaultServiceName = "OpenTelemetry Exporter";

private readonly int maxPayloadSizeInBytes;
private readonly TProtocolFactory protocolFactory;
private readonly TTransport clientTransport;
Expand Down Expand Up @@ -58,7 +57,9 @@ internal JaegerExporter(JaegerExporterOptions options, TTransport clientTranspor
this.memoryTransport = new InMemoryTransport(16000);
this.memoryProtocol = this.protocolFactory.GetProtocol(this.memoryTransport);

this.Process = new Process(DefaultServiceName, options.ProcessTags);
string serviceName = (string)this.ParentProvider.GetDefaultResource().Attributes.Where(
Austin-Tan marked this conversation as resolved.
Show resolved Hide resolved
pair => pair.Key == ResourceSemanticConventions.AttributeServiceName).FirstOrDefault().Value;
this.Process = new Process(serviceName, options.ProcessTags);
}

internal Process Process { get; set; }
Expand Down Expand Up @@ -130,14 +131,14 @@ internal void SetResourceAndInitializeBatch(Resource resource)

if (serviceName != null)
{
process.ServiceName = serviceNamespace != null
? serviceNamespace + "." + serviceName
: serviceName;
serviceName = string.IsNullOrEmpty(serviceNamespace)
? serviceName
: serviceNamespace + "." + serviceName;
}

if (string.IsNullOrEmpty(process.ServiceName))
if (!string.IsNullOrEmpty(serviceName))
{
process.ServiceName = DefaultServiceName;
process.ServiceName = serviceName;
}

this.Process.Message = this.BuildThriftMessage(this.Process).ToArray();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,6 @@ OpenTelemetry.Exporter.ZipkinExporterOptions.Endpoint.get -> System.Uri
OpenTelemetry.Exporter.ZipkinExporterOptions.Endpoint.set -> void
OpenTelemetry.Exporter.ZipkinExporterOptions.ExportProcessorType.get -> OpenTelemetry.ExportProcessorType
OpenTelemetry.Exporter.ZipkinExporterOptions.ExportProcessorType.set -> void
OpenTelemetry.Exporter.ZipkinExporterOptions.ServiceName.get -> string
OpenTelemetry.Exporter.ZipkinExporterOptions.ServiceName.set -> void
OpenTelemetry.Exporter.ZipkinExporterOptions.UseShortTraceIds.get -> bool
OpenTelemetry.Exporter.ZipkinExporterOptions.UseShortTraceIds.set -> void
OpenTelemetry.Exporter.ZipkinExporterOptions.ZipkinExporterOptions() -> void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ OpenTelemetry.Exporter.ZipkinExporterOptions.ExportProcessorType.get -> OpenTele
OpenTelemetry.Exporter.ZipkinExporterOptions.ExportProcessorType.set -> void
OpenTelemetry.Exporter.ZipkinExporterOptions.MaxPayloadSizeInBytes.get -> int?
OpenTelemetry.Exporter.ZipkinExporterOptions.MaxPayloadSizeInBytes.set -> void
OpenTelemetry.Exporter.ZipkinExporterOptions.ServiceName.get -> string
OpenTelemetry.Exporter.ZipkinExporterOptions.ServiceName.set -> void
OpenTelemetry.Exporter.ZipkinExporterOptions.UseShortTraceIds.get -> bool
OpenTelemetry.Exporter.ZipkinExporterOptions.UseShortTraceIds.set -> void
OpenTelemetry.Exporter.ZipkinExporterOptions.ZipkinExporterOptions() -> void
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ OpenTelemetry.Exporter.ZipkinExporterOptions.ExportProcessorType.get -> OpenTele
OpenTelemetry.Exporter.ZipkinExporterOptions.ExportProcessorType.set -> void
OpenTelemetry.Exporter.ZipkinExporterOptions.MaxPayloadSizeInBytes.get -> int?
OpenTelemetry.Exporter.ZipkinExporterOptions.MaxPayloadSizeInBytes.set -> void
OpenTelemetry.Exporter.ZipkinExporterOptions.ServiceName.get -> string
OpenTelemetry.Exporter.ZipkinExporterOptions.ServiceName.set -> void
OpenTelemetry.Exporter.ZipkinExporterOptions.UseShortTraceIds.get -> bool
OpenTelemetry.Exporter.ZipkinExporterOptions.UseShortTraceIds.set -> void
OpenTelemetry.Exporter.ZipkinExporterOptions.ZipkinExporterOptions() -> void
Expand Down
4 changes: 3 additions & 1 deletion src/OpenTelemetry.Exporter.Zipkin/ZipkinExporter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Net.Http.Headers;
Expand Down Expand Up @@ -119,7 +120,8 @@ internal void SetLocalEndpointFromResource(Resource resource)

if (string.IsNullOrEmpty(serviceName))
{
serviceName = this.options.ServiceName;
serviceName = (string)this.ParentProvider.GetDefaultResource().Attributes.Where(
Austin-Tan marked this conversation as resolved.
Show resolved Hide resolved
pair => pair.Key == ResourceSemanticConventions.AttributeServiceName).FirstOrDefault().Value;
}

this.LocalEndpoint = new ZipkinEndpoint(
Expand Down
10 changes: 2 additions & 8 deletions src/OpenTelemetry.Exporter.Zipkin/ZipkinExporterOptions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@

using System;
using System.Diagnostics;
using System.Linq;
using OpenTelemetry.Resources;

namespace OpenTelemetry.Exporter
{
Expand All @@ -24,18 +26,10 @@ namespace OpenTelemetry.Exporter
/// </summary>
public sealed class ZipkinExporterOptions
{
internal const string DefaultServiceName = "OpenTelemetry Exporter";

#if !NET452
internal const int DefaultMaxPayloadSizeInBytes = 4096;
#endif

/// <summary>
/// Gets or sets the name of the service reporting telemetry. If the `Resource` associated with the telemetry
/// has "service.name" defined, then it'll be preferred over this option.
/// </summary>
public string ServiceName { get; set; } = DefaultServiceName;

/// <summary>
/// Gets or sets Zipkin endpoint address. See https://zipkin.io/zipkin-api/#/default/post_spans.
/// Typically https://zipkin-server-name:9411/api/v2/spans.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ override OpenTelemetry.Trace.TraceIdRatioBasedSampler.ShouldSample(in OpenTeleme
abstract OpenTelemetry.BaseExportProcessor<T>.OnExport(T data) -> void
override sealed OpenTelemetry.BaseExportProcessor<T>.OnStart(T data) -> void
readonly OpenTelemetry.BaseExportProcessor<T>.exporter -> OpenTelemetry.BaseExporter<T>
static OpenTelemetry.ProviderExtensions.GetDefaultResource(this OpenTelemetry.BaseProvider baseProvider) -> OpenTelemetry.Resources.Resource
static OpenTelemetry.ProviderExtensions.GetResource(this OpenTelemetry.BaseProvider baseProvider) -> OpenTelemetry.Resources.Resource
static OpenTelemetry.Resources.ResourceBuilderExtensions.AddEnvironmentVariableDetector(this OpenTelemetry.Resources.ResourceBuilder resourceBuilder) -> OpenTelemetry.Resources.ResourceBuilder
static OpenTelemetry.Resources.Resource.Empty.get -> OpenTelemetry.Resources.Resource
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ override OpenTelemetry.Trace.TraceIdRatioBasedSampler.ShouldSample(in OpenTeleme
abstract OpenTelemetry.BaseExportProcessor<T>.OnExport(T data) -> void
override sealed OpenTelemetry.BaseExportProcessor<T>.OnStart(T data) -> void
readonly OpenTelemetry.BaseExportProcessor<T>.exporter -> OpenTelemetry.BaseExporter<T>
static OpenTelemetry.ProviderExtensions.GetDefaultResource(this OpenTelemetry.BaseProvider baseProvider) -> OpenTelemetry.Resources.Resource
static OpenTelemetry.ProviderExtensions.GetResource(this OpenTelemetry.BaseProvider baseProvider) -> OpenTelemetry.Resources.Resource
static OpenTelemetry.Resources.ResourceBuilderExtensions.AddEnvironmentVariableDetector(this OpenTelemetry.Resources.ResourceBuilder resourceBuilder) -> OpenTelemetry.Resources.ResourceBuilder
static OpenTelemetry.Resources.Resource.Empty.get -> OpenTelemetry.Resources.Resource
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ abstract OpenTelemetry.BaseExportProcessor<T>.OnExport(T data) -> void
override sealed OpenTelemetry.BaseExportProcessor<T>.OnStart(T data) -> void
readonly OpenTelemetry.BaseExportProcessor<T>.exporter -> OpenTelemetry.BaseExporter<T>
static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.AddOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder builder, System.Action<OpenTelemetry.Logs.OpenTelemetryLoggerOptions> configure = null) -> Microsoft.Extensions.Logging.ILoggingBuilder
static OpenTelemetry.ProviderExtensions.GetDefaultResource(this OpenTelemetry.BaseProvider baseProvider) -> OpenTelemetry.Resources.Resource
static OpenTelemetry.ProviderExtensions.GetResource(this OpenTelemetry.BaseProvider baseProvider) -> OpenTelemetry.Resources.Resource
static OpenTelemetry.Resources.ResourceBuilderExtensions.AddEnvironmentVariableDetector(this OpenTelemetry.Resources.ResourceBuilder resourceBuilder) -> OpenTelemetry.Resources.ResourceBuilder
static OpenTelemetry.Resources.Resource.Empty.get -> OpenTelemetry.Resources.Resource
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ abstract OpenTelemetry.BaseExportProcessor<T>.OnExport(T data) -> void
override sealed OpenTelemetry.BaseExportProcessor<T>.OnStart(T data) -> void
readonly OpenTelemetry.BaseExportProcessor<T>.exporter -> OpenTelemetry.BaseExporter<T>
static Microsoft.Extensions.Logging.OpenTelemetryLoggingExtensions.AddOpenTelemetry(this Microsoft.Extensions.Logging.ILoggingBuilder builder, System.Action<OpenTelemetry.Logs.OpenTelemetryLoggerOptions> configure = null) -> Microsoft.Extensions.Logging.ILoggingBuilder
static OpenTelemetry.ProviderExtensions.GetDefaultResource(this OpenTelemetry.BaseProvider baseProvider) -> OpenTelemetry.Resources.Resource
static OpenTelemetry.ProviderExtensions.GetResource(this OpenTelemetry.BaseProvider baseProvider) -> OpenTelemetry.Resources.Resource
static OpenTelemetry.Resources.ResourceBuilderExtensions.AddEnvironmentVariableDetector(this OpenTelemetry.Resources.ResourceBuilder resourceBuilder) -> OpenTelemetry.Resources.ResourceBuilder
static OpenTelemetry.Resources.Resource.Empty.get -> OpenTelemetry.Resources.Resource
Expand Down
3 changes: 3 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@

* Default `Resource` will now contain service.name instead of Telemetry SDK.
[#1744](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1744)
* Exporters fall back to the default `Resource`'s `service.name` value instead
Austin-Tan marked this conversation as resolved.
Show resolved Hide resolved
of their own presets. Providers extended to access the default `Resource`.
[#1768](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1768)

## 1.0.0-rc2

Expand Down
10 changes: 10 additions & 0 deletions src/OpenTelemetry/ProviderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -38,5 +38,15 @@ public static Resource GetResource(this BaseProvider baseProvider)

return Resource.Empty;
}

/// <summary>
/// Gets the <see cref="Resource"/> associated with the <see cref="BaseProvider"/>.
/// </summary>
/// <param name="baseProvider"><see cref="BaseProvider"/>.</param>
/// <returns><see cref="Resource"/>if found otherwise <see cref="Resource.Empty"/>.</returns>
public static Resource GetDefaultResource(this BaseProvider baseProvider)
{
return ResourceBuilder.CreateDefault().Build();
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ namespace OpenTelemetry.Exporter.Jaeger.Tests
{
public class JaegerExporterTests
{
private const string DefaultServiceName = "OpenTelemetry Exporter";

[Fact]
public void JaegerExporter_BadArgs()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,6 @@ public void SuppresssesInstrumentation()

var exporterOptions = new ZipkinExporterOptions
{
ServiceName = "test-zipkin",
Endpoint = new Uri($"http://{this.testServerHost}:{this.testServerPort}/api/v2/spans?requestId={requestId}"),
};
var zipkinExporter = new ZipkinExporter(exporterOptions);
Expand Down Expand Up @@ -171,8 +170,9 @@ public void IntegrationTest(
UseShortTraceIds = useShortTraceIds,
});

var serviceName = ZipkinExporterOptions.DefaultServiceName;
var resoureTags = string.Empty;
var serviceName = (string)exporter.ParentProvider.GetDefaultResource().Attributes
.Where(pair => pair.Key == ResourceSemanticConventions.AttributeServiceName).FirstOrDefault().Value;
var resourceTags = string.Empty;
var activity = CreateTestActivity(isRootSpan: isRootSpan, status: status);
if (useTestResource)
{
Expand Down Expand Up @@ -237,7 +237,7 @@ public void IntegrationTest(
}

Assert.Equal(
$@"[{{""traceId"":""{traceId}"",""name"":""Name"",{parentId}""id"":""{ZipkinActivityConversionExtensions.EncodeSpanId(context.SpanId)}"",""kind"":""CLIENT"",""timestamp"":{timestamp},""duration"":60000000,""localEndpoint"":{{""serviceName"":""{serviceName}""{ipInformation}}},""remoteEndpoint"":{{""serviceName"":""http://localhost:44312/""}},""annotations"":[{{""timestamp"":{eventTimestamp},""value"":""Event1""}},{{""timestamp"":{eventTimestamp},""value"":""Event2""}}],""tags"":{{{resoureTags}""stringKey"":""value"",""longKey"":""1"",""longKey2"":""1"",""doubleKey"":""1"",""doubleKey2"":""1"",""longArrayKey"":""1,2"",""boolKey"":""true"",""boolArrayKey"":""true,false"",""http.host"":""http://localhost:44312/"",{statusTag}""otel.library.name"":""CreateTestActivity"",""peer.service"":""http://localhost:44312/""{errorTag}}}}}]",
$@"[{{""traceId"":""{traceId}"",""name"":""Name"",{parentId}""id"":""{ZipkinActivityConversionExtensions.EncodeSpanId(context.SpanId)}"",""kind"":""CLIENT"",""timestamp"":{timestamp},""duration"":60000000,""localEndpoint"":{{""serviceName"":""{serviceName}""{ipInformation}}},""remoteEndpoint"":{{""serviceName"":""http://localhost:44312/""}},""annotations"":[{{""timestamp"":{eventTimestamp},""value"":""Event1""}},{{""timestamp"":{eventTimestamp},""value"":""Event2""}}],""tags"":{{{resourceTags}""stringKey"":""value"",""longKey"":""1"",""longKey2"":""1"",""doubleKey"":""1"",""doubleKey2"":""1"",""longArrayKey"":""1,2"",""boolKey"":""true"",""boolArrayKey"":""true,false"",""http.host"":""http://localhost:44312/"",{statusTag}""otel.library.name"":""CreateTestActivity"",""peer.service"":""http://localhost:44312/""{errorTag}}}}}]",
Responses[requestId]);
}

Expand Down