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

Upgrade Semantic Convention handling in AWS libraries. #2367

Open
wants to merge 30 commits into
base: main
Choose a base branch
from

Conversation

ppittle
Copy link
Member

@ppittle ppittle commented Dec 6, 2024

Fixes #
Design discussion issue #

NOTE: This PR is built on top of #2363, #2363 should be merged first.

Changes

Implementation of the design discussed in the Nov 26th OpenTelemetry .NET SIG meeting. This design allows releasing the AWS OTel Contrib libraries as GA while still informing users that underlying Semantic Conventions are experimental.

This is exposed via the SemanticConventionVersion enum:

/// <summary>
/// <para>
/// Collection of the Open Telemetry Semantic Conventions supported by the OpenTelemetry.*.AWS libraries.
/// Can be used to pin the version of Semantic Convention emitted.
/// </para>
/// <para>
/// While these libraries are intended for production use, they rely on several Semantic Conventions that are
/// still considered Experimental, meaning they may undergo additional changes before becoming Stable.  This can
/// impact the aggregation and analysis of telemetry signals in environments with multiple applications or microservices.
/// For example, a microservice using an older version of the Semantic Conventions for Http Attributes may emit
/// <c>"http.method"</c> with a value of GET,while a different microservice, using a new version of Semantic Convention may instead emit the GET as
/// <c>"http.request.method"</c>.
/// </para>
/// <para>
/// Future versions the OpenTelemetry.*.AWS libraries will include updates to the Semantic Convention, which may break compatibility with a previous version.
/// To opt-out of automatic upgrades, you can pin to a specific version:
/// <code>
/// <![CDATA[
///  using (var tracerProvider = Sdk.CreateTracerProviderBuilder()
///     .AddAWSLambdaConfigurations(opt =>
///     {
///         opt.SemanticConventionVersion = SemanticConventionVersion.v1_10_EXPERIMENTAL;
///     })
///     .Build()!)
/// ]]>
/// </code>
/// </para>
/// <para>
/// For additional details, see:
/// https://opentelemetry.io/docs/specs/otel/versioning-and-stability/.
/// </para>
/// </summary>
/// <remarks>
/// Once a Semantic Convention becomes Stable, OpenTelemetry.*.AWS libraries will remain on that version until the
/// next major version bump.
/// </remarks>
public enum SemanticConventionVersion
{
    /// <summary>
    /// Use Experimental Conventions until they become stable and then
    /// pin to stable.
    /// </summary>
    Latest = 0,

    /// <summary>
    /// Pin to the specific state of all Semantic Conventions as of the 1.27.0
    /// release. See:
    /// https://github.com/open-telemetry/semantic-conventions/releases/tag/v1.27.0.
    /// </summary>
    v1_27_0_Experimental = 1,

    /// <summary>
    /// Pin to the specific state of all Semantic Conventions as of the 1.29.0
    /// release. See:
    /// https://github.com/open-telemetry/semantic-conventions/releases/tag/v1.29.0.
    /// </summary>
    v1_29_0_Experimental = 2,
}

Users can choose how to handle this for their applications by either pinning to a specific Semantic Convention version or automatically getting updates the next time they upgrade the AWS Otel nuget packages.

Selection is made by updating the Options object in the builder method.

using (var tracerProvider = Sdk.CreateTracerProviderBuilder()
           .AddAWSLambdaConfigurations(opt =>
           {
               // Added in this PR
               opt.SemanticConventionVersion = SemanticConventionVersion.v1_10_Experimental ;
           })
           .AddInMemoryExporter(exportedItems)
           .Build()!)

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Unit tests added/updated
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • Changes in public API reviewed (if applicable)

@ppittle ppittle requested a review from a team as a code owner December 6, 2024 04:25
@github-actions github-actions bot added comp:extensions.aws Things related to OpenTelemetry.Extensions.AWS comp:instrumentation.aws Things related to OpenTelemetry.Instrumentation.AWS comp:instrumentation.awslambda Things related to OpenTelemetry.Instrumentation.AWSLambda comp:resources.aws Things related to OpenTelemetry.Resources.AWS labels Dec 6, 2024
Copy link

codecov bot commented Dec 6, 2024

Codecov Report

Attention: Patch coverage is 88.51675% with 24 lines in your changes missing coverage. Please review.

Project coverage is 71.69%. Comparing base (71655ce) to head (21379d0).
Report is 651 commits behind head on main.

Files with missing lines Patch % Lines
...etry.Resources.AWS/AWSResourceBuilderExtensions.cs 0.00% 16 Missing ⚠️
src/OpenTelemetry.Resources.AWS/AWSECSDetector.cs 93.18% 3 Missing ⚠️
...on.AWS/Implementation/AWSTracingPipelineHandler.cs 91.66% 1 Missing ⚠️
...ntation.AWSLambda/Implementation/AWSLambdaUtils.cs 94.44% 1 Missing ⚠️
src/OpenTelemetry.Resources.AWS/AWSEBSDetector.cs 93.33% 1 Missing ⚠️
src/OpenTelemetry.Resources.AWS/AWSEC2Detector.cs 94.11% 1 Missing ⚠️
...lemetry.Resources.AWS/AWSResourceBuilderOptions.cs 0.00% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2367      +/-   ##
==========================================
- Coverage   73.91%   71.69%   -2.23%     
==========================================
  Files         267      368     +101     
  Lines        9615    14216    +4601     
==========================================
+ Hits         7107    10192    +3085     
- Misses       2508     4024    +1516     
Flag Coverage Δ
unittests-Contrib.Shared.Tests 84.04% <ø> (?)
unittests-Exporter.Geneva 58.87% <ø> (?)
unittests-Exporter.InfluxDB 95.14% <ø> (?)
unittests-Exporter.Instana 74.86% <ø> (?)
unittests-Exporter.OneCollector 94.57% <ø> (?)
unittests-Exporter.Stackdriver 79.26% <ø> (?)
unittests-Extensions 88.63% <ø> (?)
unittests-Extensions.Enrichment 100.00% <ø> (?)
unittests-Instrumentation.AWS 86.56% <98.05%> (?)
unittests-Instrumentation.AspNet 77.00% <ø> (?)
unittests-Instrumentation.AspNetCore 70.33% <ø> (?)
unittests-Instrumentation.ConfluentKafka 14.37% <ø> (?)
unittests-Instrumentation.ElasticsearchClient 80.12% <ø> (?)
unittests-Instrumentation.EntityFrameworkCore 57.06% <ø> (?)
unittests-Instrumentation.EventCounters 76.36% <ø> (?)
unittests-Instrumentation.GrpcCore 91.42% <ø> (?)
unittests-Instrumentation.GrpcNetClient 79.61% <ø> (?)
unittests-Instrumentation.Hangfire 93.58% <ø> (?)
unittests-Instrumentation.Http 74.33% <ø> (?)
unittests-Instrumentation.Owin 88.12% <ø> (?)
unittests-Instrumentation.Process 100.00% <ø> (?)
unittests-Instrumentation.Quartz 78.76% <ø> (?)
unittests-Instrumentation.Runtime 100.00% <ø> (?)
unittests-Instrumentation.SqlClient 88.38% <ø> (?)
unittests-Instrumentation.StackExchangeRedis 68.26% <ø> (?)
unittests-Instrumentation.Wcf 49.55% <ø> (?)
unittests-PersistentStorage 65.88% <ø> (?)
unittests-Resources.AWS 75.83% <79.24%> (?)
unittests-Resources.Azure 85.03% <ø> (?)
unittests-Resources.Container 67.34% <ø> (?)
unittests-Resources.Gcp 71.15% <ø> (?)
unittests-Resources.Host 73.91% <ø> (?)
unittests-Resources.OperatingSystem 76.98% <ø> (?)
unittests-Resources.Process 100.00% <ø> (?)
unittests-Resources.ProcessRuntime 78.26% <ø> (?)
unittests-Sampler.AWS 88.12% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...rumentation.AWS/AWSClientInstrumentationOptions.cs 100.00% <100.00%> (ø)
...rumentation.AWS/Implementation/AWSServiceHelper.cs 98.71% <100.00%> (-1.29%) ⬇️
...ation.AWSLambda/AWSLambdaInstrumentationOptions.cs 100.00% <100.00%> (ø)
...etry.Instrumentation.AWSLambda/AWSLambdaWrapper.cs 94.91% <100.00%> (+0.08%) ⬆️
...ion.AWSLambda/Implementation/AWSLambdaHttpUtils.cs 100.00% <100.00%> (ø)
...Lambda/Implementation/AWSLambdaResourceDetector.cs 100.00% <100.00%> (ø)
...ation.AWSLambda/Implementation/CommonExtensions.cs 71.42% <ø> (-8.58%) ⬇️
...ation.AWSLambda/TracerProviderBuilderExtensions.cs 100.00% <100.00%> (ø)
src/OpenTelemetry.Resources.AWS/AWSEKSDetector.cs 65.51% <100.00%> (ø)
...on.AWS/Implementation/AWSTracingPipelineHandler.cs 88.46% <91.66%> (+0.70%) ⬆️
... and 6 more

... and 366 files with indirect coverage changes

@ppittle ppittle force-pushed the upgrade-aws-semantic-convention branch from da63599 to 3183104 Compare December 6, 2024 04:45
@ppittle ppittle force-pushed the upgrade-aws-semantic-convention branch from 3183104 to ddef75b Compare December 6, 2024 04:55
@github-actions github-actions bot removed the comp:extensions.aws Things related to OpenTelemetry.Extensions.AWS label Dec 6, 2024
@ppittle ppittle force-pushed the upgrade-aws-semantic-convention branch from ddef75b to f62eb8d Compare December 6, 2024 04:58
@ppittle ppittle force-pushed the upgrade-aws-semantic-convention branch from f62eb8d to 59b59f1 Compare December 6, 2024 05:11
@ppittle ppittle force-pushed the upgrade-aws-semantic-convention branch from 59b59f1 to ed49ef0 Compare December 6, 2024 06:19
@ppittle ppittle force-pushed the upgrade-aws-semantic-convention branch from ed49ef0 to 1dbdb19 Compare December 6, 2024 23:10
ppittle and others added 22 commits December 13, 2024 13:08
…ire some additional changes in the Instrumentation.AWSLambda project to accomidate using url.path and url.query instead.
…ent version used by the library. Users must opt-in to newer versions of semantic conventions.
Co-authored-by: Alan West <3676547+alanwest@users.noreply.github.com>
@ppittle ppittle force-pushed the upgrade-aws-semantic-convention branch from 68b455e to bc393a5 Compare December 13, 2024 21:08
@ppittle ppittle requested a review from Kielek December 13, 2024 21:10
ppittle and others added 3 commits December 13, 2024 15:00
Co-authored-by: Piotr Kiełkowicz <pkiekowicz@splunk.com>
 Semantinc Convention enums
@ppittle ppittle force-pushed the upgrade-aws-semantic-convention branch from bc393a5 to 21379d0 Compare December 13, 2024 23:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:instrumentation.aws Things related to OpenTelemetry.Instrumentation.AWS comp:instrumentation.awslambda Things related to OpenTelemetry.Instrumentation.AWSLambda comp:resources.aws Things related to OpenTelemetry.Resources.AWS
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants