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.EntityFrameworkCore] Update semantic conventions for stable release #2130

Merged
merged 13 commits into from
Oct 17, 2024
Merged
1 change: 1 addition & 0 deletions opentelemetry-dotnet-contrib.sln
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,7 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Shared", "Shared", "{1FCC8E
ProjectSection(SolutionItems) = preProject
src\Shared\ActivityInstrumentationHelper.cs = src\Shared\ActivityInstrumentationHelper.cs
src\Shared\AssemblyVersionExtensions.cs = src\Shared\AssemblyVersionExtensions.cs
src\Shared\DatabaseSemanticConventionHelper.cs = src\Shared\DatabaseSemanticConventionHelper.cs
src\Shared\DiagnosticSourceListener.cs = src\Shared\DiagnosticSourceListener.cs
src\Shared\DiagnosticSourceSubscriber.cs = src\Shared\DiagnosticSourceSubscriber.cs
src\Shared\ExceptionExtensions.cs = src\Shared\ExceptionExtensions.cs
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,28 @@

## Unreleased

* The new database semantic conventions can be opted in to by setting
the `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable. This allows for a
transition period for users to experiment with the new semantic conventions
and adapt as necessary. The environment variable supports the following
values:
* `database` - emit the new, frozen (proposed for stable) database
attributes, and stop emitting the old experimental database
attributes that the instrumentation emitted previously.
* `database/dup` - emit both the old and the frozen (proposed for stable) database
attributes, allowing for a more seamless transition.
* The default behavior (in the absence of one of these values) is to continue
emitting the same database semantic conventions that were emitted in
the previous version.
* Note: this option will be be removed after the new database
semantic conventions is marked stable. At which time this
instrumentation can receive a stable release, and the old database
semantic conventions will no longer be supported. Refer to the
specification for more information regarding the new database
semantic conventions for
[spans](https://github.com/open-telemetry/semantic-conventions/blob/v1.28.0/docs/database/database-spans.md).
([#2130](https://github.com/open-telemetry/opentelemetry-dotnet-contrib/pull/2130))

## 1.0.0-beta.12

Released 2024-Jun-18
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

using System.Data;
using System.Diagnostics;
using Microsoft.Extensions.Configuration;
using static OpenTelemetry.Internal.DatabaseSemanticConventionHelper;

namespace OpenTelemetry.Instrumentation.EntityFrameworkCore;

Expand All @@ -11,6 +13,21 @@ namespace OpenTelemetry.Instrumentation.EntityFrameworkCore;
/// </summary>
public class EntityFrameworkInstrumentationOptions
{
/// <summary>
/// Initializes a new instance of the <see cref="EntityFrameworkInstrumentationOptions"/> class.
/// </summary>
public EntityFrameworkInstrumentationOptions()
: this(new ConfigurationBuilder().AddEnvironmentVariables().Build())
{
}

internal EntityFrameworkInstrumentationOptions(IConfiguration configuration)
{
var databaseSemanticConvention = GetSemanticConventionOptIn(configuration);
this.EmitOldAttributes = databaseSemanticConvention.HasFlag(DatabaseSemanticConvention.Old);
this.EmitNewAttributes = databaseSemanticConvention.HasFlag(DatabaseSemanticConvention.New);
}

/// <summary>
/// Gets or sets a value indicating whether or not the <see cref="EntityFrameworkInstrumentation"/> should add the names of <see cref="CommandType.StoredProcedure"/> commands as the <see cref="Implementation.EntityFrameworkDiagnosticListener.AttributeDbStatement"/> tag. Default value: True.
/// </summary>
Expand Down Expand Up @@ -50,4 +67,14 @@ public class EntityFrameworkInstrumentationOptions
/// </list>
/// </remarks>
public Func<string?, IDbCommand, bool>? Filter { get; set; }

/// <summary>
/// Gets or sets a value indicating whether the old database attributes should be emitted.
/// </summary>
internal bool EmitOldAttributes { get; set; }

/// <summary>
/// Gets or sets a value indicating whether the new database attributes should be emitted.
/// </summary>
internal bool EmitNewAttributes { get; set; }
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,12 @@ internal sealed class EntityFrameworkDiagnosticListener : ListenerHandler
internal const string EntityFrameworkCoreCommandError = "Microsoft.EntityFrameworkCore.Database.Command.CommandError";

internal const string AttributePeerService = "peer.service";
internal const string AttributeServerAddress = "server.address";
internal const string AttributeDbSystem = "db.system";
internal const string AttributeDbName = "db.name";
internal const string AttributeDbNamespace = "db.namespace";
internal const string AttributeDbStatement = "db.statement";
internal const string AttributeDbQueryText = "db.query.text";

internal static readonly Assembly Assembly = typeof(EntityFrameworkDiagnosticListener).Assembly;
internal static readonly string ActivitySourceName = Assembly.GetName().Name;
Expand Down Expand Up @@ -139,10 +142,19 @@ public override void OnEventWritten(string name, object? payload)
}

var dataSource = (string)this.dataSourceFetcher.Fetch(connection);
activity.AddTag(AttributeDbName, database);
alanwest marked this conversation as resolved.
Show resolved Hide resolved
if (!string.IsNullOrEmpty(dataSource))
{
activity.AddTag(AttributePeerService, dataSource);
activity.AddTag(AttributeServerAddress, dataSource);
}

if (this.options.EmitOldAttributes)
{
activity.AddTag(AttributeDbName, database);
}

if (this.options.EmitNewAttributes)
{
activity.AddTag(AttributeDbNamespace, database);
Copy link
Member

@alanwest alanwest Oct 16, 2024

Choose a reason for hiding this comment

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

@trask @lmolkova what are your thoughts here?

Per footnote 4 in the conventions:

Semantic conventions for individual database systems SHOULD document what db.namespace means in the context of that system.

Since this instrumentation is for an ORM and supports many database systems, this would ideally respect the conventions declared for the target database system though I don't know if that will be possible.

Also not all systems this instrumentation supports have documentation for what db.namespace means.

Thoughts on ORMs in general?

Copy link
Member

Choose a reason for hiding this comment

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

we have a similar issue in Java (https://github.com/open-telemetry/opentelemetry-java-instrumentation/blob/main/instrumentation/jdbc/library/src/test/java/io/opentelemetry/instrumentation/jdbc/internal/JdbcConnectionUrlParserTest.java)

technically, I think stable JDBC instrumentation should only emit stable instrumentation by default, which means only emitting instrumentation for stable db.system values

in practice, this would mean that we can't mark the JDBC instrumentation as stable even after we update it to emit the stable database semconv (unless we want to force lots of users to opt-in to the unstable db.system values, which I don't think is a great experience)

I'm hoping this will motivate us to push semconv definitions for db.namespace for all of these other databases quickly, we just haven't had the bandwidth to do it yet and didn't want to block initial database semconv stability on it

}
}
}
Expand Down Expand Up @@ -196,15 +208,31 @@ public override void OnEventWritten(string name, object? payload)
case CommandType.StoredProcedure:
if (this.options.SetDbStatementForStoredProcedure)
{
activity.AddTag(AttributeDbStatement, commandText);
if (this.options.EmitOldAttributes)
{
activity.AddTag(AttributeDbStatement, commandText);
}

if (this.options.EmitNewAttributes)
{
activity.AddTag(AttributeDbQueryText, commandText);
}
}

break;

case CommandType.Text:
if (this.options.SetDbStatementForText)
{
activity.AddTag(AttributeDbStatement, commandText);
if (this.options.EmitOldAttributes)
{
activity.AddTag(AttributeDbStatement, commandText);
}

if (this.options.EmitNewAttributes)
{
activity.AddTag(AttributeDbQueryText, commandText);
}
}

break;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,28 @@
<MinVerTagPrefix>Instrumentation.EntityFrameworkCore-</MinVerTagPrefix>
</PropertyGroup>

<!-- Do not run Package Baseline Validation as this package has never released a stable version.
<!-- Do not run Package Baseline Validation as this package has never released a stable version.
Remove this property once we have released a stable version and add PackageValidationBaselineVersion property. -->
<PropertyGroup>
<DisablePackageBaselineValidation>true</DisablePackageBaselineValidation>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="Microsoft.Extensions.Configuration" Version="$(MicrosoftExtensionsConfigurationPkgVer)"/>
<PackageReference Include="Microsoft.Extensions.Options" Version="$(MicrosoftExtensionsOptionsPkgVer)" />
<PackageReference Include="OpenTelemetry.Api.ProviderBuilderExtensions" Version="$(OpenTelemetryCoreLatestVersion)" />
</ItemGroup>

<ItemGroup>
<Compile Include="$(RepoRoot)\src\Shared\AssemblyVersionExtensions.cs" Link="Includes\AssemblyVersionExtensions.cs" />
<Compile Include="$(RepoRoot)\src\Shared\DatabaseSemanticConventionHelper.cs" Link="Includes\DatabaseSemanticConventionHelper.cs" />
<Compile Include="$(RepoRoot)\src\Shared\DiagnosticSourceListener.cs" Link="Includes\DiagnosticSourceListener.cs" />
<Compile Include="$(RepoRoot)\src\Shared\DiagnosticSourceSubscriber.cs" Link="Includes\DiagnosticSourceSubscriber.cs" />
<Compile Include="$(RepoRoot)\src\Shared\EnvironmentVariables\*.cs" Link="Includes\EnvironmentVariables\%(Filename).cs" />
<Compile Include="$(RepoRoot)\src\Shared\ExceptionExtensions.cs" Link="Includes\ExceptionExtensions.cs" />
<Compile Include="$(RepoRoot)\src\Shared\Guard.cs" Link="Includes\Guard.cs" />
<Compile Include="$(RepoRoot)\src\Shared\ListenerHandler.cs" Link="Includes\ListenerHandler.cs" />
<Compile Include="$(RepoRoot)\src\Shared\NullableAttributes.cs" Link="Includes\NullableAttributes.cs" />
<Compile Include="$(RepoRoot)\src\Shared\PropertyFetcher.cs" Link="Includes\PropertyFetcher.cs" />
</ItemGroup>

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

#nullable enable

using System.Diagnostics.CodeAnalysis;
using Microsoft.Extensions.Configuration;

namespace OpenTelemetry.Internal;

/// <summary>
/// Helper class for Database Semantic Conventions.
/// </summary>
/// <remarks>
/// Due to a breaking change in the semantic convention, affected instrumentation libraries
/// must inspect an environment variable to determine which attributes to emit.
/// This is expected to be removed when the instrumentation libraries reach Stable.
/// <see href="https://github.com/open-telemetry/semantic-conventions/blob/v1.28.0/docs/database/database-spans.md"/>.
/// </remarks>
internal static class DatabaseSemanticConventionHelper
{
internal const string SemanticConventionOptInKeyName = "OTEL_SEMCONV_STABILITY_OPT_IN";
internal static readonly char[] Separator = new[] { ',', ' ' };

[Flags]
public enum DatabaseSemanticConvention
{
/// <summary>
/// Instructs an instrumentation library to emit the old experimental Database attributes.
/// </summary>
Old = 0x1,

/// <summary>
/// Instructs an instrumentation library to emit the new, v1.28.0 Database attributes.
/// </summary>
New = 0x2,

/// <summary>
/// Instructs an instrumentation library to emit both the old and new attributes.
/// </summary>
Dupe = Old | New,
}

public static DatabaseSemanticConvention GetSemanticConventionOptIn(IConfiguration configuration)
{
if (TryGetConfiguredValues(configuration, out var values))
{
if (values.Contains("database/dup"))
{
return DatabaseSemanticConvention.Dupe;
}
else if (values.Contains("database"))
{
return DatabaseSemanticConvention.New;
}
}

return DatabaseSemanticConvention.Old;
}

private static bool TryGetConfiguredValues(IConfiguration configuration, [NotNullWhen(true)] out HashSet<string>? values)
{
try
{
var stringValue = configuration[SemanticConventionOptInKeyName];

if (string.IsNullOrWhiteSpace(stringValue))
{
values = null;
return false;
}

var stringValues = stringValue!.Split(separator: Separator, options: StringSplitOptions.RemoveEmptyEntries);
values = new HashSet<string>(stringValues, StringComparer.OrdinalIgnoreCase);
return true;
}
catch
{
values = null;
return false;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

using Microsoft.Extensions.Configuration;
using Xunit;
using static OpenTelemetry.Internal.DatabaseSemanticConventionHelper;

namespace OpenTelemetry.Internal.Tests;

public class DatabaseSemanticConventionHelperTests
joegoldman2 marked this conversation as resolved.
Show resolved Hide resolved
{
public static IEnumerable<object[]> TestCases => new List<object[]>
{
new object[] { null!, DatabaseSemanticConvention.Old },
new object[] { string.Empty, DatabaseSemanticConvention.Old },
new object[] { " ", DatabaseSemanticConvention.Old },
new object[] { "junk", DatabaseSemanticConvention.Old },
new object[] { "none", DatabaseSemanticConvention.Old },
new object[] { "NONE", DatabaseSemanticConvention.Old },
new object[] { "database", DatabaseSemanticConvention.New },
new object[] { "DATABASE", DatabaseSemanticConvention.New },
new object[] { "database/dup", DatabaseSemanticConvention.Dupe },
new object[] { "DATABASE/DUP", DatabaseSemanticConvention.Dupe },
new object[] { "junk,,junk", DatabaseSemanticConvention.Old },
new object[] { "junk,JUNK", DatabaseSemanticConvention.Old },
new object[] { "junk1,junk2", DatabaseSemanticConvention.Old },
new object[] { "junk,database", DatabaseSemanticConvention.New },
new object[] { "junk,database , database ,junk", DatabaseSemanticConvention.New },
new object[] { "junk,database/dup", DatabaseSemanticConvention.Dupe },
new object[] { "junk, database/dup ", DatabaseSemanticConvention.Dupe },
new object[] { "database/dup,database", DatabaseSemanticConvention.Dupe },
new object[] { "database,database/dup", DatabaseSemanticConvention.Dupe },
};

[Fact]
public void VerifyFlags()
{
var testValue = DatabaseSemanticConvention.Dupe;
Assert.True(testValue.HasFlag(DatabaseSemanticConvention.Old));
Assert.True(testValue.HasFlag(DatabaseSemanticConvention.New));

testValue = DatabaseSemanticConvention.Old;
Assert.True(testValue.HasFlag(DatabaseSemanticConvention.Old));
Assert.False(testValue.HasFlag(DatabaseSemanticConvention.New));

testValue = DatabaseSemanticConvention.New;
Assert.False(testValue.HasFlag(DatabaseSemanticConvention.Old));
Assert.True(testValue.HasFlag(DatabaseSemanticConvention.New));
}

[Theory]
[MemberData(nameof(TestCases))]
public void VerifyGetSemanticConventionOptIn_UsingEnvironmentVariable(string input, string expectedValue)
{
try
{
Environment.SetEnvironmentVariable(SemanticConventionOptInKeyName, input);

var expected = Enum.Parse(typeof(DatabaseSemanticConvention), expectedValue);
Assert.Equal(expected, GetSemanticConventionOptIn(new ConfigurationBuilder().AddEnvironmentVariables().Build()));
}
finally
{
Environment.SetEnvironmentVariable(SemanticConventionOptInKeyName, null);
}
}

[Theory]
[MemberData(nameof(TestCases))]
public void VerifyGetSemanticConventionOptIn_UsingIConfiguration(string input, string expectedValue)
{
var configuration = new ConfigurationBuilder()
.AddInMemoryCollection(new Dictionary<string, string?> { [SemanticConventionOptInKeyName] = input })
.Build();

var expected = Enum.Parse(typeof(DatabaseSemanticConvention), expectedValue);
Assert.Equal(expected, GetSemanticConventionOptIn(configuration));
}
}
Loading
Loading