DB and HTTP semantic convention stability migration for Redis instrumentation by tammy-baylis-swi · Pull Request #4370 · open-telemetry/opentelemetry-python-contrib

tammy-baylis-swi · 2026-03-27T23:49:07Z

Description

Add Redis instrumentor support for when OTEL_SEMCONV_STABILITY_OPT_IN includes

"database" or "database/dup"
"http" or "http/dup"

EDIT: This uses now-merged updates from SQLAlchemy PR, no conflicts 🙂 ~~This duplicates all _semconv helper updates in #4110 at 02adc40~~

Fixes #2930, #2885

Type of change

Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Added unit tests

Does This PR Require a Core Repo Change?

Yes. - Link to PR:
No.

Checklist:

See contributing.md for styleguide, changelog guidelines, and more.

Followed the style guidelines of this project
Changelogs have been updated
Unit tests have been added
Documentation has been updated

tammy-baylis-swi · 2026-03-30T19:05:13Z

 pluggy==1.6.0
 py-cpuinfo==9.0.0
 pytest==7.4.4
+pytest-asyncio==0.23.5


Added in 1c7e3cc so Python 3.9 tests pass, else RuntimeError: There is no current event loop in thread 'MainThread'.

do we still need that after dropping py39?

Nope! 🎉 Removed in 961bab7

MikeGoldsmith

Looks good - thanks @tammy-baylis-swi.

Left some queries about some semconv keys and a couple of suggestions.

MikeGoldsmith · 2026-04-08T14:03:26Z

+    )
+
    db = conn_kwargs.get("db", 0)
    attributes[DB_REDIS_DATABASE_INDEX] = db


I think db.redis.database_index became db.namespace - do we have a set helper that can use old/new/dual write for these?

Sorry it's taken me a while to get back into this one!

@MikeGoldsmith it looks like db.redis.database_index is deprecated with no replacement in semconv 1.38.0 (PR). So my interpretation is that old semconv should write the old attribute, and new should write no attribute. While the attribute is only used by the Redis instrumentation, I've put a helper in _semconv.py in 1d9f2a0 for consistency. It does not set any attribute if "new" semconv. Lmk what you think!

MikeGoldsmith · 2026-04-08T14:04:15Z

+        _set_http_net_peer_name_client(
+            attributes, conn_kwargs.get("path", ""), http_sem_conv_opt_in_mode
+        )
        attributes[NET_TRANSPORT] = NetTransportValues.OTHER.value


I think net.transport has been deprecated - should we drop this in stable mode?

yeah. maybe just set if _report_old()

In 54881b8 I've added _set_net_transport helper for old/new/dup. I think the current trail from old net.transport goes to "deprecated" network.transport then "final" network.transport, so it's got both a _report_old() and _report_new(). Lmk your thoughts!

MikeGoldsmith · 2026-04-08T14:04:24Z

+            conn_kwargs.get("port", 6379),
+            http_sem_conv_opt_in_mode,
+        )
        attributes[NET_TRANSPORT] = NetTransportValues.IP_TCP.value


Same as above.

Relates to 54881b8

MikeGoldsmith · 2026-04-08T14:10:18Z

            self.assertEqual(span.status.status_code, trace.StatusCode.UNSET)
+
+
+class TestRedisSemconvConfiguration(TestRedis):


Do we need to inherit TestRedis or could we use TestBase? I think we'll re-run all the stuff in TestRedis like this.

I think we're missing the async tests for _async_traced_execute_factory and _async_traced_execute_pipeline_factory too.

Addressed in 732d1e7!

MikeGoldsmith · 2026-04-08T14:13:09Z

    response_hook: ResponseHook | None = None,
 ):
+    # Get semconv opt-in modes for database and HTTP signal types
+    _OpenTelemetrySemanticConventionStability._initialize()


These are duplicated across all four factories, should we add a helper?

Good call! Added helper in b1fde84 within Redis instrumentation.

If this gets merged I may put in another PR later to house it in _semconv.py because SQLAlchemy instrumentation could benefit from it too (duplication added in this PR).

emdneto · 2026-04-09T00:17:16Z

+        self.assertEqual(len(spans), 1)
+
+        span = spans[0]
+        self.assertIn(DB_STATEMENT, span.attributes)


can we assert db.system.name?

I think addressed in 23280ae plus more asserts in 34de467 but GH has been fussy for me all day 😵 LMK if needed in a different test function too.

emdneto · 2026-05-05T00:00:25Z

 pluggy==1.6.0
 py-cpuinfo==9.0.0
 pytest==7.4.4
+pytest-asyncio==0.23.5


do we still need that after dropping py39?

tammy-baylis-swi · 2026-05-05T00:39:25Z

Please pardon my delay in addressing your feedback @emdneto and @MikeGoldsmith ! Please could you take another pass through this one for Redis instrumentation and semconv opt-in.

Current CI/CD failures for precommit "rstcheck" seem unrelated and present in other PRs:

rstcheck.................................................................Failed
- hook id: rstcheck
- exit code: 1

_template/autodoc_entry.rst:1: (SEVERE/4) File referenced in "include" directive not found: '../../../instrumentation/opentelemetry-instrumentation-<REPLACE ME>/README.rst'.

emdneto · 2026-05-05T02:30:21Z

Please pardon my delay in addressing your feedback @emdneto and @MikeGoldsmith ! Please could you take another pass through this one for Redis instrumentation and semconv opt-in.

Current CI/CD failures for precommit "rstcheck" seem unrelated and present in other PRs:
rstcheck.................................................................Failed
- hook id: rstcheck
- exit code: 1

_template/autodoc_entry.rst:1: (SEVERE/4) File referenced in "include" directive not found: '../../../instrumentation/opentelemetry-instrumentation-<REPLACE ME>/README.rst'.

Np. I realized I left a pending review since last month 😅 Looking into the rstcheck error

tammy-baylis-swi added 6 commits March 27, 2026 16:44

Duplicate: add _semconv schem_url helper

02adc40

Redis Http,Database semconv opt in

6cc6600

Changelog

269949b

semconv status generated

aadf1d2

lint

1d4d6d5

Add dep for py39 redis test

1c7e3cc

tammy-baylis-swi commented Mar 30, 2026

View reviewed changes

tammy-baylis-swi marked this pull request as ready for review March 30, 2026 19:14

tammy-baylis-swi requested a review from a team as a code owner March 30, 2026 19:14

tammy-baylis-swi mentioned this pull request Mar 30, 2026

feat(redis): support semantic convention opt-in and emit stable Redis attributes #3826

Closed

6 tasks

tammy-baylis-swi added this to Python PR digest Mar 30, 2026

tammy-baylis-swi moved this to Ready for review in Python PR digest Mar 30, 2026

tammy-baylis-swi added 3 commits April 1, 2026 17:22

Merge branch 'main' into redis-semconv-opt-in

1a033fb

Merge branch 'main' into redis-semconv-opt-in

fafc229

Merge branch 'main' into redis-semconv-opt-in

ef149b3

MikeGoldsmith reviewed Apr 8, 2026

View reviewed changes

Merge branch 'main' into redis-semconv-opt-in

e2f5f0f

tammy-baylis-swi moved this from Ready for review to Reviewed PRs that need fixes in Python PR digest Apr 23, 2026

tammy-baylis-swi and others added 5 commits April 30, 2026 15:13

Merge branch 'main' into redis-semconv-opt-in

f2c0940

Merge branch 'main' into redis-semconv-opt-in

ea83a81

Add _set_db_redis_database_index

1d9f2a0

Add _set_net_transport

54881b8

Add _get_semconv_opt_in_modes

b1fde84

emdneto reviewed May 5, 2026

View reviewed changes

tammy-baylis-swi added 5 commits May 4, 2026 17:11

TestBase and async coverage

732d1e7

Add more attr asserts

34de467

Add test_pipeline attr asserts

23280ae

Rm dep not needed since no py39

961bab7

pylint

19ebb5b

tammy-baylis-swi added 2 commits May 5, 2026 10:59

Merge branch 'main' into redis-semconv-opt-in

cf2d4dd

Merge branch 'main' into redis-semconv-opt-in

93a9f89

		self.assertEqual(span.status.status_code, trace.StatusCode.UNSET)


		class TestRedisSemconvConfiguration(TestRedis):

Conversation

tammy-baylis-swi commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

How Has This Been Tested?

Does This PR Require a Core Repo Change?

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MikeGoldsmith left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tammy-baylis-swi commented May 5, 2026

Uh oh!

emdneto commented May 5, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

tammy-baylis-swi commented Mar 27, 2026 •

edited

Loading