opentelemetry-instrumentation-boto3sqs: migrate to current messaging semantic conventions - #4920
opentelemetry-instrumentation-boto3sqs: migrate to current messaging semantic conventions#4920anneheartrecord wants to merge 3 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 778f72b085
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| operation_name: str, | ||
| operation_type: MessagingOperationTypeValues, |
There was a problem hiding this comment.
Removing queue_url from _enrich_span drops the previous endpoint information without replacing it with the current convention's server.address (and server.port for non-default ports), even though every instrumented call already supplies the queue URL. As a result, spans for AWS endpoints and especially LocalStack/custom endpoints can no longer identify the server that handled the operation, and the declared v1.27 messaging telemetry is incomplete; retain and parse the URL using the generated server attribute constants.
AGENTS.md reference: AGENTS.md:L84-L87
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch — added server.address (and server.port when the URL carries an explicit port, e.g. LocalStack) parsed from the queue URL in _enrich_span, replacing the endpoint info the old messaging.url attribute provided. Covered by the updated default-attrs assertions plus a new custom-endpoint test. 5804e8c
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5804e8c649
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if parsed_url.port: | ||
| span.set_attribute(SERVER_PORT, parsed_url.port) |
There was a problem hiding this comment.
When a custom QueueUrl contains a nonnumeric or out-of-range port, accessing parsed_url.port raises ValueError before the wrapped botocore operation runs. Previously this value was forwarded so botocore could produce its own validation or request error; the instrumentation now changes application behavior instead of preserving the underlying exception. Skip the server attributes when URL parsing fails.
AGENTS.md reference: AGENTS.md:L80-L82
Useful? React with 👍 / 👎.
| if parsed_url.port: | ||
| span.set_attribute(SERVER_PORT, parsed_url.port) |
There was a problem hiding this comment.
When a valid queue URL explicitly includes its scheme's default port, such as https://host:443/... or http://host:80/..., this truthiness check emits server.port. The current server semantic convention requires omitting the attribute for the default port, so the newly added endpoint enrichment produces nonconforming telemetry for these URLs; compare the parsed scheme and only set non-default ports.
Useful? React with 👍 / 👎.
1 participant
Description
Migrates the boto3sqs instrumentation off the deprecated
opentelemetry.semconv.trace.SpanAttributes(schema 1.11.0) onto the current incubating messaging attributes, following the pattern already used by the aiokafka instrumentation:messaging.system:aws.sqs→aws_sqs(MessagingSystemValues.AWS_SQS)messaging.destination→messaging.destination.namemessaging.operation→messaging.operation.name(send/receive/process) +messaging.operation.type; send spans now carry operation attributes toomessaging.destination_kindandmessaging.url(removed from the spec)messaging.conversation_id, which was being set to thesend_message_batchentryId(not a conversation/message-group id per spec)Schemas.V1_27_0(same as aiokafka)Span names are intentionally left as
{queue} {operation}to match the existing aiokafka naming and avoid an extra breaking change; happy to adjust if you'd rather move to{operation} {queue}here as well.Fixes #1639
Type of change
How Has This Been Tested?
tests/test_boto3sqs_instrumentation.pyto assert the new attributes; full suite passes locally (16 passed) with boto3/botocore 1.34.44 (the pinned test versions), plusruff check/ruff format --check.Does This PR Require a Core Repo Change?
Checklist: