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

Default Resource should have service.name attribute #1744

Merged
merged 21 commits into from
Feb 1, 2021
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
Expand Up @@ -120,6 +120,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 OpenTelemetry.ProviderExtensions.GetResource(this OpenTelemetry.BaseProvider baseProvider) -> OpenTelemetry.Resources.Resource
static OpenTelemetry.Resources.ResourceBuilderExtensions.AddDefault(this OpenTelemetry.Resources.ResourceBuilder resourceBuilder) -> OpenTelemetry.Resources.ResourceBuilder
static OpenTelemetry.Resources.ResourceBuilderExtensions.AddEnvironmentVariableDetector(this OpenTelemetry.Resources.ResourceBuilder resourceBuilder) -> OpenTelemetry.Resources.ResourceBuilder
static OpenTelemetry.Resources.Resource.Empty.get -> OpenTelemetry.Resources.Resource
static OpenTelemetry.Resources.ResourceBuilderExtensions.AddAttributes(this OpenTelemetry.Resources.ResourceBuilder resourceBuilder, System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, object>> attributes) -> OpenTelemetry.Resources.ResourceBuilder
Expand Down
1 change: 1 addition & 0 deletions src/OpenTelemetry/.publicApi/net46/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,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 OpenTelemetry.ProviderExtensions.GetResource(this OpenTelemetry.BaseProvider baseProvider) -> OpenTelemetry.Resources.Resource
static OpenTelemetry.Resources.ResourceBuilderExtensions.AddDefault(this OpenTelemetry.Resources.ResourceBuilder resourceBuilder) -> OpenTelemetry.Resources.ResourceBuilder
static OpenTelemetry.Resources.ResourceBuilderExtensions.AddEnvironmentVariableDetector(this OpenTelemetry.Resources.ResourceBuilder resourceBuilder) -> OpenTelemetry.Resources.ResourceBuilder
static OpenTelemetry.Resources.Resource.Empty.get -> OpenTelemetry.Resources.Resource
static OpenTelemetry.Resources.ResourceBuilderExtensions.AddAttributes(this OpenTelemetry.Resources.ResourceBuilder resourceBuilder, System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, object>> attributes) -> OpenTelemetry.Resources.ResourceBuilder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ 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.GetResource(this OpenTelemetry.BaseProvider baseProvider) -> OpenTelemetry.Resources.Resource
static OpenTelemetry.Resources.ResourceBuilderExtensions.AddDefault(this OpenTelemetry.Resources.ResourceBuilder resourceBuilder) -> OpenTelemetry.Resources.ResourceBuilder
static OpenTelemetry.Resources.ResourceBuilderExtensions.AddEnvironmentVariableDetector(this OpenTelemetry.Resources.ResourceBuilder resourceBuilder) -> OpenTelemetry.Resources.ResourceBuilder
static OpenTelemetry.Resources.Resource.Empty.get -> OpenTelemetry.Resources.Resource
static OpenTelemetry.Resources.ResourceBuilderExtensions.AddAttributes(this OpenTelemetry.Resources.ResourceBuilder resourceBuilder, System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, object>> attributes) -> OpenTelemetry.Resources.ResourceBuilder
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ 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.GetResource(this OpenTelemetry.BaseProvider baseProvider) -> OpenTelemetry.Resources.Resource
static OpenTelemetry.Resources.ResourceBuilderExtensions.AddDefault(this OpenTelemetry.Resources.ResourceBuilder resourceBuilder) -> OpenTelemetry.Resources.ResourceBuilder
static OpenTelemetry.Resources.ResourceBuilderExtensions.AddEnvironmentVariableDetector(this OpenTelemetry.Resources.ResourceBuilder resourceBuilder) -> OpenTelemetry.Resources.ResourceBuilder
static OpenTelemetry.Resources.Resource.Empty.get -> OpenTelemetry.Resources.Resource
static OpenTelemetry.Resources.ResourceBuilderExtensions.AddAttributes(this OpenTelemetry.Resources.ResourceBuilder resourceBuilder, System.Collections.Generic.IEnumerable<System.Collections.Generic.KeyValuePair<string, object>> attributes) -> OpenTelemetry.Resources.ResourceBuilder
Expand Down
4 changes: 4 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@
* Metrics removed as it is not part 1.0.0 release. See issue
[#1501](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1655)
for details on Metric release plans.
* Default `Resource` has been updated. No longer linked to Telemetry SDK,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a simple one liner like this is sufficient for changelog: Default resource will now contain service.name instead of telemetrysdk.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. Also the line above it, issue #1501 redirects to PR #1655. is this intended?

but that enabled through .AddTelemetrySDK() extensions. Instead provides
default service.name attribute composed of local service and process name.
[#1744](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1744)

## 1.0.0-rc1.1

Expand Down
8 changes: 4 additions & 4 deletions src/OpenTelemetry/Resources/ResourceBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,14 +31,14 @@ private ResourceBuilder()
}

/// <summary>
/// Creates a <see cref="ResourceBuilder"/> instance with SDK defaults
/// added. See <a
/// href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/resource/semantic_conventions/">resource
/// Creates a <see cref="ResourceBuilder"/> instance with Default
/// service.name added. See <a
/// href="https://github.com/open-telemetry/opentelemetry-specification/tree/master/specification/resource/semantic_conventions#service/">resource
Austin-Tan marked this conversation as resolved.
Show resolved Hide resolved
/// semantic conventions</a> for details.
/// </summary>
/// <returns>Created <see cref="ResourceBuilder"/>.</returns>
public static ResourceBuilder CreateDefault()
=> new ResourceBuilder().AddTelemetrySdk(); // TODO: Seek spec clarify on whether or not OtelEnvResourceDetector should be added by default.
=> new ResourceBuilder().AddDefault();

/// <summary>
/// Creates an empty <see cref="ResourceBuilder"/> instance.
Expand Down
21 changes: 20 additions & 1 deletion src/OpenTelemetry/Resources/ResourceBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ public static class ResourceBuilderExtensions
{
private static readonly Version Version = typeof(Resource).Assembly.GetName().Version;

private static Resource DefaultResource { get; } = new Resource(new Dictionary<string, object>
{
[ResourceSemanticConventions.AttributeServiceName] = "unknown_service:"
+ (string.IsNullOrWhiteSpace(System.Diagnostics.Process.GetCurrentProcess().ProcessName) ? System.Diagnostics.Process.GetCurrentProcess().ProcessName : string.Empty),
});

private static Resource TelemetryResource { get; } = new Resource(new Dictionary<string, object>
{
[ResourceSemanticConventions.AttributeTelemetrySdkName] = "opentelemetry",
Expand Down Expand Up @@ -89,7 +95,20 @@ public static ResourceBuilder AddService(
/// <summary>
/// Adds service information to a <see cref="ResourceBuilder"/>
/// following <a
/// href="https://github.com/open-telemetry/opentelemetry-specification/tree/main/specification/resource/semantic_conventions#telemetry-sdk">semantic
/// href="https://github.com/open-telemetry/opentelemetry-specification/tree/master/specification/resource/semantic_conventions#service">semantic
/// conventions</a>.
/// </summary>
/// <param name="resourceBuilder"><see cref="ResourceBuilder"/>.</param>
/// <returns>Returns <see cref="ResourceBuilder"/> for chaining.</returns>
public static ResourceBuilder AddDefault(this ResourceBuilder resourceBuilder)
Austin-Tan marked this conversation as resolved.
Show resolved Hide resolved
{
return resourceBuilder.AddResource(DefaultResource);
}

/// <summary>
/// Adds service information to a <see cref="ResourceBuilder"/>
/// following <a
/// href="https://github.com/open-telemetry/opentelemetry-specification/tree/master/specification/resource/semantic_conventions#telemetry-sdk">semantic
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this change is incorrect. the branch was named to "main" last week.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed this, but when I use .../master/... the link doesn't properly direct to the right part of the page. /main/ will show the warning but also direct to the right section of the page.
I'll still change it to master.

/// conventions</a>.
/// </summary>
/// <param name="resourceBuilder"><see cref="ResourceBuilder"/>.</param>
Expand Down
31 changes: 25 additions & 6 deletions test/OpenTelemetry.Tests/Resources/ResourceTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,18 @@ public void MergeResource_UpdatingResourceOverridesCurrentResource()
Assert.Contains(new KeyValuePair<string, object>("value", "updatedValue"), newResource.Attributes);
}

[Fact]
public void GetResourceWithTelemetrySDKAttributes()
{
// Arrange
var resource = ResourceBuilder.CreateDefault().AddTelemetrySdk().AddEnvironmentVariableDetector().Build();

// Assert
var attributes = resource.Attributes;
Assert.Equal(4, attributes.Count());
ValidateTelemetrySdkAttributes(attributes);
}

[Fact]
public void GetResourceWithDefaultAttributes_EmptyResource()
{
Expand All @@ -338,8 +350,8 @@ public void GetResourceWithDefaultAttributes_EmptyResource()

// Assert
var attributes = resource.Attributes;
Assert.Equal(3, attributes.Count());
ValidateTelemetrySdkAttributes(attributes);
Assert.Single(attributes);
ValidateDefaultAttributes(attributes);
}

[Fact]
Expand All @@ -350,9 +362,9 @@ public void GetResourceWithDefaultAttributes_ResourceWithAttrs()

// Assert
var attributes = resource.Attributes;
Assert.Equal(5, attributes.Count());
Assert.Equal(3, attributes.Count());
ValidateAttributes(attributes, 0, 1);
ValidateTelemetrySdkAttributes(attributes);
ValidateDefaultAttributes(attributes);
}

[Fact]
Expand All @@ -364,9 +376,9 @@ public void GetResourceWithDefaultAttributes_WithEnvVar()

// Assert
var attributes = resource.Attributes;
Assert.Equal(7, attributes.Count());
Assert.Equal(5, attributes.Count());
ValidateAttributes(attributes, 0, 1);
ValidateTelemetrySdkAttributes(attributes);
ValidateDefaultAttributes(attributes);
Assert.Contains(new KeyValuePair<string, object>("EVKey1", "EVVal1"), attributes);
Assert.Contains(new KeyValuePair<string, object>("EVKey2", "EVVal2"), attributes);
}
Expand Down Expand Up @@ -412,6 +424,13 @@ private static void ValidateTelemetrySdkAttributes(IEnumerable<KeyValuePair<stri
Assert.Single(versionAttribute);
}

private static void ValidateDefaultAttributes(IEnumerable<KeyValuePair<string, object>> attributes)
{
var serviceName = attributes.Where(pair => pair.Key.Equals("service.name"));
Assert.Single(serviceName);
Assert.Contains("unknown_service:", serviceName.FirstOrDefault().Value as string);
}

private Dictionary<string, object> CreateAttributes(int attributeCount, int startIndex = 0)
{
var attributes = new Dictionary<string, object>();
Expand Down
5 changes: 3 additions & 2 deletions test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -369,9 +369,10 @@ public void TracerProvideSdkCreatesAndDiposesInstrumentation()
}

[Fact]
public void TracerProviderSdkBuildsWithDefaultResource()
public void TracerProviderSdkBuildsWithSDKResource()
Austin-Tan marked this conversation as resolved.
Show resolved Hide resolved
{
var tracerProvider = Sdk.CreateTracerProviderBuilder().Build();
var tracerProvider = Sdk.CreateTracerProviderBuilder().SetResourceBuilder(
ResourceBuilder.CreateDefault().AddTelemetrySdk()).Build();
var resource = tracerProvider.GetResource();
var attributes = resource.Attributes;

Expand Down