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

[Instrumentation.GrpNetClient, Instrumentation.SqlCliet] set ActivitySource.Version to NuGet package version #5498

Merged
merged 18 commits into from
Apr 9, 2024
Merged
Changes from 3 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
1 change: 1 addition & 0 deletions OpenTelemetry.sln
Original file line number Diff line number Diff line change
@@ -272,6 +272,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Shared", "Shared", "{A49299
src\Shared\RequestMethodHelper.cs = src\Shared\RequestMethodHelper.cs
src\Shared\ResourceSemanticConventions.cs = src\Shared\ResourceSemanticConventions.cs
src\Shared\SemanticConventions.cs = src\Shared\SemanticConventions.cs
src\Shared\SignalVersionHelper.cs = src\Shared\SignalVersionHelper.cs
src\Shared\SpanAttributeConstants.cs = src\Shared\SpanAttributeConstants.cs
src\Shared\SpanHelper.cs = src\Shared\SpanHelper.cs
src\Shared\StatusHelper.cs = src\Shared\StatusHelper.cs
Original file line number Diff line number Diff line change
@@ -2,6 +2,9 @@

## Unreleased

* `ActivitySource.Version` is set to NuGet package version.
([#5498](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5498))

## 1.7.0-beta.1

Released 2024-Feb-09
Original file line number Diff line number Diff line change
@@ -16,8 +16,8 @@ internal sealed class GrpcClientDiagnosticListener : ListenerHandler
{
internal static readonly AssemblyName AssemblyName = typeof(GrpcClientDiagnosticListener).Assembly.GetName();
internal static readonly string ActivitySourceName = AssemblyName.Name;
internal static readonly Version Version = AssemblyName.Version;
internal static readonly ActivitySource ActivitySource = new(ActivitySourceName, Version.ToString());
internal static readonly string Version = SignalVersionHelper.GetVersion<GrpcClientDiagnosticListener>();
Copy link
Member

Choose a reason for hiding this comment

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

The PR title and changelog both say this is NuGet package version, the code here doesn't seem to be NuGet version (I guess there is some assumption that the portion of assembly version would be the same as the package version, not sure if that's something we can rely on though), could you explain how this would work?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure if you have chance to review: open-telemetry/opentelemetry-dotnet-contrib#1624. I will try to description behavior once more time.

  1. Before changes, InstrumentationScope.Version was reported as Assmebly.Version.
  2. Assembly.Version by this recommendation typically is set to nonzero.0.0.0. In all instrumentation packages it return 1.0.0.0. In general it is almost useless value, as we have tons of different behaviors in such scope.
  3. Both, this repository and contrib are using MinVer for versioning libraries and nuget packages.
  4. MinVer together with SourceLink (commit part) produces for commit 33d5521 for SqlClient instrumentation library following values: (last release/sql tag was set to 1.7.0-beta.1)
    • NuGet version: 1.7.0-beta.1.86
    • AssemblyInformationalVersionAttribute 1.7.0-beta.1.86+33d5521a73e881ac59d4bf1213765270ec2422ff (this is in format {nuget-version}-{commithash}.
  5. NuGet version is in sync with all other versions

IMO, for end users, describing new value as a NuGet version is the easiest option. Relaying on AssemblyInformationalVersionAttribute is more problematic.

internal static readonly ActivitySource ActivitySource = new(ActivitySourceName, Version);

private const string OnStartEvent = "Grpc.Net.Client.GrpcOut.Start";
private const string OnStopEvent = "Grpc.Net.Client.GrpcOut.Stop";
Original file line number Diff line number Diff line change
@@ -14,6 +14,7 @@
<ItemGroup>
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Instrumentation.Http\HttpRequestMessageContextPropagation.cs" Link="Includes\HttpRequestMessageContextPropagation.cs" />
<Compile Include="$(RepoRoot)\src\Shared\Guard.cs" Link="Includes\Guard.cs" />
<Compile Include="$(RepoRoot)\src\Shared\SignalVersionHelper.cs" Link="Includes\SignalVersionHelper.cs" />
<Compile Include="$(RepoRoot)\src\Shared\Options\*.cs" Link="Includes\Options\%(Filename).cs" />
<Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\Shims\NullableAttributes.cs" />
</ItemGroup>
3 changes: 3 additions & 0 deletions src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -2,6 +2,9 @@

## Unreleased

* `ActivitySource.Version` is set to NuGet package version.
([#5498](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5498))

## 1.7.0-beta.1

Released 2024-Feb-09
Original file line number Diff line number Diff line change
@@ -17,8 +17,7 @@ internal sealed class SqlActivitySourceHelper

public static readonly AssemblyName AssemblyName = typeof(SqlActivitySourceHelper).Assembly.GetName();
public static readonly string ActivitySourceName = AssemblyName.Name;
public static readonly Version Version = AssemblyName.Version;
public static readonly ActivitySource ActivitySource = new(ActivitySourceName, Version.ToString());
public static readonly ActivitySource ActivitySource = new(ActivitySourceName, SignalVersionHelper.GetVersion<SqlActivitySourceHelper>());
public static readonly string ActivityName = ActivitySourceName + ".Execute";

public static readonly IEnumerable<KeyValuePair<string, object>> CreationTags = new[]
Original file line number Diff line number Diff line change
@@ -13,6 +13,7 @@

<ItemGroup>
<Compile Include="$(RepoRoot)\src\Shared\Guard.cs" Link="Includes\Guard.cs" />
<Compile Include="$(RepoRoot)\src\Shared\SignalVersionHelper.cs" Link="Includes\SignalVersionHelper.cs" />
<Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\Shims\NullableAttributes.cs" />
</ItemGroup>

14 changes: 14 additions & 0 deletions src/Shared/SignalVersionHelper.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

using System.Reflection;

namespace OpenTelemetry.Instrumentation;

internal static class SignalVersionHelper
Kielek marked this conversation as resolved.
Show resolved Hide resolved
{
public static string GetVersion<T>()
Kielek marked this conversation as resolved.
Show resolved Hide resolved
{
return typeof(T).Assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>()!.InformationalVersion.Split('+')[0];
}
}
Original file line number Diff line number Diff line change
@@ -435,7 +435,7 @@ public void Grpc_BadArgs()
private static void ValidateGrpcActivity(Activity activityToValidate)
{
Assert.Equal(GrpcClientDiagnosticListener.ActivitySourceName, activityToValidate.Source.Name);
Assert.Equal(GrpcClientDiagnosticListener.Version.ToString(), activityToValidate.Source.Version);
Assert.Equal(GrpcClientDiagnosticListener.Version, activityToValidate.Source.Version);
Assert.Equal(ActivityKind.Client, activityToValidate.Kind);
}
}