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 12 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
4 changes: 4 additions & 0 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ Released 2021-Jan-29
[#1501](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1655)
for details on Metric release plans.
* Fix Resource attribute telemetry.sdk.version to have correct file version.
* 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
15 changes: 11 additions & 4 deletions src/OpenTelemetry/Resources/ResourceBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,22 @@ private ResourceBuilder()
{
}

private static Resource DefaultResource { get; } = new Resource(new Dictionary<string, object>
{
[ResourceSemanticConventions.AttributeServiceName] = "unknown_service:"
Austin-Tan marked this conversation as resolved.
Show resolved Hide resolved
+ (string.IsNullOrWhiteSpace(System.Diagnostics.Process.GetCurrentProcess().ProcessName)
? System.Diagnostics.Process.GetCurrentProcess().ProcessName : string.Empty),
});

/// <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().AddResource(DefaultResource);

/// <summary>
/// Creates an empty <see cref="ResourceBuilder"/> instance.
Expand Down
2 changes: 1 addition & 1 deletion src/OpenTelemetry/Resources/ResourceBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ 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#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
16 changes: 16 additions & 0 deletions test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -375,6 +375,22 @@ public void TracerProviderSdkBuildsWithDefaultResource()
var resource = tracerProvider.GetResource();
var attributes = resource.Attributes;

Assert.NotNull(resource);
Assert.NotEqual(Resource.Empty, resource);
Assert.Single(resource.Attributes);
Assert.Equal(resource.Attributes.FirstOrDefault().Key, ResourceSemanticConventions.AttributeServiceName);
Assert.Contains("unknown_service:", (string)resource.Attributes.FirstOrDefault().Value);

}

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

Assert.NotNull(resource);
Assert.NotEqual(Resource.Empty, resource);
Assert.Contains(new KeyValuePair<string, object>("telemetry.sdk.name", "opentelemetry"), attributes);
Expand Down