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.

Sign up for GitHub

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

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

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
a71856c
Include SignalVersionHelper
Kielek Apr 2, 2024
a5ae3fc
Instrumentation.SqlClient
Kielek Apr 2, 2024
af3e76e
Instrumentation.GrpcNetClient
Kielek Apr 2, 2024
6c21efd
Merge branch 'main' into instrumentation-scope-version
Kielek Apr 3, 2024
5585b56
Merge branch 'main' into instrumentation-scope-version
Kielek Apr 3, 2024
33d5521
SignalVersionHelper -> InstrumentationScopeHelper
Kielek Apr 4, 2024
4294446
Add behavior description for version logic
Kielek Apr 4, 2024
7d538b2
Merge branch 'main' into instrumentation-scope-version
Kielek Apr 5, 2024
0e6b2ae
PR feedback
Kielek Apr 5, 2024
fa7bae9
PR feedback - optimisation
Kielek Apr 5, 2024
0d8add2
Merge branch 'main' into instrumentation-scope-version
Kielek Apr 8, 2024
9f5f5bf
PR feedback - change fetching version contract
Kielek Apr 8, 2024
5d382f0
Remove reference to non-existing file
Kielek Apr 8, 2024
d8f9a1a
PR feedback unify taking version in SDK
Kielek Apr 8, 2024
7b3104d
Adjust tests
Kielek Apr 8, 2024
2e48de8
Merge branch 'main' into instrumentation-scope-version
vishweshbankwar Apr 8, 2024
15a4721
Merge branch 'main' into instrumentation-scope-version
Kielek Apr 8, 2024
bd3b88b
Merge branch 'main' into instrumentation-scope-version
Kielek Apr 9, 2024
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Shared", "Shared", "{A49299
src\Shared\DiagnosticDefinitions.cs = src\Shared\DiagnosticDefinitions.cs
src\Shared\ExceptionExtensions.cs = src\Shared\ExceptionExtensions.cs
src\Shared\Guard.cs = src\Shared\Guard.cs
src\Shared\InstrumentationScopeHelper..cs = src\Shared\InstrumentationScopeHelper..cs
src\Shared\MathHelper.cs = src\Shared\MathHelper.cs
src\Shared\PeerServiceResolver.cs = src\Shared\PeerServiceResolver.cs
src\Shared\PeriodicExportingMetricReaderHelper.cs = src\Shared\PeriodicExportingMetricReaderHelper.cs
Expand Down
3 changes: 3 additions & 0 deletions src/OpenTelemetry.Instrumentation.GrpcNetClient/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions ...penTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = InstrumentationScopeHelper.GetVersion<GrpcClientDiagnosticListener>();
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";
Expand Down
1 change: 1 addition & 0 deletions ...elemetry.Instrumentation.GrpcNetClient/OpenTelemetry.Instrumentation.GrpcNetClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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\InstrumentationScopeHelper.cs" Link="Includes\InstrumentationScopeHelper.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>
Expand Down
3 changes: 3 additions & 0 deletions src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlActivitySourceHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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, InstrumentationScopeHelper.GetVersion<SqlActivitySourceHelper>());
public static readonly string ActivityName = ActivitySourceName + ".Execute";

public static readonly IEnumerable<KeyValuePair<string, object>> CreationTags = new[]
Expand Down
1 change: 1 addition & 0 deletions src/OpenTelemetry.Instrumentation.SqlClient/OpenTelemetry.Instrumentation.SqlClient.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

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

Expand Down
19 changes: 19 additions & 0 deletions src/Shared/InstrumentationScopeHelper.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

using System.Reflection;

namespace OpenTelemetry.Instrumentation;

internal static class InstrumentationScopeHelper
Copy link
Member

Choose a reason for hiding this comment

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

@Kielek We have similar logic here:

opentelemetry-dotnet/src/OpenTelemetry/Sdk.cs

Lines 34 to 35 in 3f4c73a

var assemblyInformationalVersion = typeof(Sdk).Assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>()?.InformationalVersion;
InformationalVersion = ParseAssemblyInformationalVersion(assemblyInformationalVersion);

opentelemetry-dotnet/src/OpenTelemetry/Sdk.cs

Line 114 in 3f4c73a

internal static string ParseAssemblyInformationalVersion(string? informationalVersion)

Also here but it seems a little different:

opentelemetry-dotnet/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/OtlpExporterOptions.cs

Line 232 in 3f4c73a

var assemblyVersion = typeof(OtlpExporterOptions).Assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>();

This new helper class doesn't really have anything to do with instrumentation scopes. Could we perhaps rename it something like AssemblyVersionHelper and refactor the SDK version stuff (maybe OtlpExporter stuff too) so that everything is using the same code for versioning?

Doesn't have to be on this PR.

The API could also be an extension method. Something like...

internal static class AssemblyVersionExtensions
{
    public static string GetPackageVersion(this Assembly assembly)
    {
        // not shown
    }
}

In the current signature you pass a type to use to lookup the assembly. I don't think that is immediately obvious IMO passing the assembly to use is nicer/more explicit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most of the changes covered in 9f5f5bf and d8f9a1a.

I think that changes in OTLP exported, related to dropping hash commit, deservers CHANGELOG entry.
It will bring as a bit closer to recommendation, to put there just version, without any additional data such as commit hash.
I will create follow up PR, when this one will be merged.

Copy link
Member

Choose a reason for hiding this comment

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

@Kielek - What will change with respect to OTLP exporter? Could you elaborate?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

User agent. Now it contains haskcommit.

Before changes OTel-OTLP-Exporter-Dotnet/version+hashcommit
After-changes OTel-OTLP-Exporter-Dotnet/version

{
public static string GetVersion<T>()
Kielek marked this conversation as resolved.
Show resolved Hide resolved
{
// MinVer https://github.com/adamralph/minver?tab=readme-ov-file#version-numbers
// together with Microsoft.SourceLink.GitHub https://github.com/dotnet/sourcelink
// fills tAssemblyInformationalVersionAttribute by
// `{NuGetPackageVersion}-{CommitHash}`, e.g. `1.7.0-beta.1.86+33d5521a73e881ac59d4bf1213765270ec2422ff`.
Kielek marked this conversation as resolved.
Show resolved Hide resolved
// For Scope version version without commit hash is returned.
Kielek marked this conversation as resolved.
Show resolved Hide resolved
return typeof(T).Assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>()!.InformationalVersion.Split('+')[0];
Kielek marked this conversation as resolved.
Show resolved Hide resolved
}
}
2 changes: 1 addition & 1 deletion test/OpenTelemetry.Instrumentation.Grpc.Tests/GrpcTests.client.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}