Skip to content

Update OpenTelemetry.Instrumentation.StackExchangeRedis - #18274

Merged
Sébastien Ros (sebastienros) merged 2 commits into
microsoft:mainfrom
martincostello:update-vendored-OTel-StackExchangeRedis
Jun 17, 2026
Merged

Update OpenTelemetry.Instrumentation.StackExchangeRedis#18274
Sébastien Ros (sebastienros) merged 2 commits into
microsoft:mainfrom
martincostello:update-vendored-OTel-StackExchangeRedis

Conversation

@martincostello

Copy link
Copy Markdown
Contributor

Description

Updated vendored code for OpenTelemetry.Instrumentation.StackExchangeRedis to Instrumentation.StackExchangeRedis-1.15.1-beta.2 as I noticed it was quite old.

Checklist

  • Is this feature complete?
    • Yes. Ready to ship.
    • No. Follow-up changes expected.
  • Are you including unit tests for the changes and scenario tests if relevant?
    • Yes
    • No
  • Did you add public API?
    • Yes
      • If yes, did you have an API Review for it?
        • Yes
        • No
      • Did you add <remarks /> and <code /> elements on your triple slash comments?
        • Yes
        • No
    • No
  • Does the change make any security assumptions or guarantees?
    • Yes
      • If yes, have you done a threat model and had a security review?
        • Yes
        • No
    • No

@github-actions

github-actions Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

🚀 Dogfood this PR with:

⚠️ WARNING: Do not do this without first carefully reviewing the code of this PR to satisfy yourself it is safe.

curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 18274

Or

  • Run remotely in PowerShell:
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 18274"

@martincostello

Martin Costello (martincostello) commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

I don't think the failing CI jobs are related to this change.

@github-actions

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@sebastienros

Copy link
Copy Markdown
Contributor

PR Testing Report

PR Information

Artifact Version Verification

  • Expected Commit: 2fc9069
  • Installed CLI Version: 13.5.0-pr.18274.g2fc9069d
  • Status: ✅ Verified (matches PR head)

Changes Analyzed

  • Client/Component: Aspire.StackExchange.Redis re-vendored to OpenTelemetry Instrumentation.StackExchangeRedis-1.15.1-beta.2.
  • Adds Microsoft.Extensions.Configuration.EnvironmentVariables package.
  • AddSource(...ActivitySourceName)AddSource(...ActivitySource.Name).
  • New semantic-convention opt-in via OTEL_SEMCONV_STABILITY_OPT_IN.

Change Categories

  • Client/Component changes
  • CLI / Hosting / Dashboard / Template / CI changes

Test Method

A console harness referencing the PR-built Aspire.StackExchange.Redis (13.5.0-pr.18274.g2fc9069d) from the PR hive was run against a real redis:7 container. It wires builder.AddRedisClient("cache") (the exact PR code path), attaches an ActivityListener, issues SET/GET, and dumps each Redis span's source name, version, and tags. This gives precise, reproducible span inspection. The harness was run for each OTEL_SEMCONV_STABILITY_OPT_IN value.

Test Scenarios Executed

Scenario 1: Redis tracing happy path + source name (validates review finding #1)

Coverage Type: Happy path
Status: ✅ Passed (behavior confirmed)

Default run (no opt-in) produced 2 Redis spans:

SOURCE_NAME='Aspire.StackExchange.Redis' SOURCE_VERSION='13.5.0-pr.18274.g2fc9069d' DISPLAY='SET'
    db.redis.database_index = 0
    db.statement = SET
    db.system = redis
    server.address = localhost
    server.port = 63799
(GET identical)

Observation: The emitted trace source name is Aspire.StackExchange.Redis, NOT OpenTelemetry.Instrumentation.StackExchangeRedis. This empirically confirms review finding #1: ActivitySourceFactory.Create<T> derives the name from the (Aspire) assembly. The source version is also now the Aspire package version. Tracing itself works correctly. Default attribute set remains the old conventions (db.statement, db.system).

Scenario 2: New semantic-convention opt-in

Coverage Type: Feature behavior
Status: ✅ Passed

  • OTEL_SEMCONV_STABILITY_OPT_IN=database → new attributes only: db.namespace, db.operation.name, db.query.text, db.system.name (no db.statement/db.system).
  • OTEL_SEMCONV_STABILITY_OPT_IN=database/dup → both old and new attributes emitted together.

The new DatabaseSemanticConventionHelper opt-in path works as designed.

Unhappy path: invalid opt-in value

Coverage Type: Negative / boundary
Status: ✅ Passed

  • OTEL_SEMCONV_STABILITY_OPT_IN=garbage → no crash; instrumentation safely falls back to the old attribute set (db.statement, db.system). 2 spans still captured.

Other Observations

  • server.address / server.port (new network attributes) are emitted in all modes including the default/old mode. This matches the upstream OTel 1.15.1-beta.2 behavior and is part of the version bump, not an Aspire-specific change.
  • Unrelated build issue: aspire start on the generated aspire-starter app failed building RedisTrace.Web/Components/_Imports.razor (CS0234: namespace 'Components' does not exist in 'RedisTrace.Web'). This is a Blazor/template namespace compile issue independent of this PR (the Redis packages restored fine and the failure is in unrelated Razor code). It did not affect Redis-instrumentation validation, which used the harness against a real Redis container.

Summary

Scenario Status Notes
Redis tracing happy path + source name ✅ Passed Source name confirmed = Aspire.StackExchange.Redis
New semantic-convention opt-in (database, database/dup) ✅ Passed New attributes emitted correctly
Invalid opt-in value (garbage) ✅ Passed Safe fallback to old attributes, no crash
Generated starter app aspire start ⚠️ Unrelated failure Blazor template namespace error, not PR-related

Overall Result

✅ PR FUNCTIONALLY VERIFIED — the updated Redis instrumentation works correctly across default and opt-in semantic conventions, with safe handling of invalid input.

Note for author

Testing confirmed the trace ActivitySource name changes from OpenTelemetry.Instrumentation.StackExchangeRedis to Aspire.StackExchange.Redis (because the vendored code is compiled into the Aspire assembly and the README customization that previously hardcoded the name was removed). Tracing still works, but this is a user-visible source-name change with no test coverage. See the inline review comment on StackExchangeRedisConnectionInstrumentation.cs. If unintended, pass an explicit name: "OpenTelemetry.Instrumentation.StackExchangeRedis" to ActivitySourceFactory.Create.

Override the name to match upstream rather than the identity of the assembly vendoring the code.
@github-actions

Copy link
Copy Markdown
Contributor

Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt.

@sebastienros
Sébastien Ros (sebastienros) merged commit 458661f into microsoft:main Jun 17, 2026
673 of 677 checks passed
@github-actions github-actions Bot added this to the 13.5 milestone Jun 17, 2026
@martincostello
Martin Costello (martincostello) deleted the update-vendored-OTel-StackExchangeRedis branch June 17, 2026 18:11
@github-actions github-actions Bot locked and limited conversation to collaborators Jul 18, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants