Repository navigation
Update vendored ConfluentKafka instrumentation to 0.3.0-alpha.4 - #19319
Sébastien Ros (sebastienros) wants to merge 8 commits into
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 19319Or
iex "& { $(irm https://raw.githubusercontent.com/microsoft/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 19319" |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Updates vendored Confluent Kafka instrumentation to 0.2.0-alpha.2 and OpenTelemetry messaging conventions v1.43.
Changes:
- Updates Kafka spans, metrics, attributes, and exception telemetry.
- Aligns Aspire’s legacy Kafka meter with renamed signals.
- Updates tests, telemetry documentation, and vendoring instructions.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
tests/Aspire.Confluent.Kafka.Tests/OtelTracesTests.cs |
Verifies renamed spans and kinds. |
tests/Aspire.Confluent.Kafka.Tests/OtelMetricsTests.cs |
Verifies new metric names. |
tests/Aspire.Confluent.Kafka.Tests/Aspire8MetricsTests.cs |
Updates legacy metric assertions. |
src/Vendoring/README.md |
Updates the vendored version and adaptations. |
src/Vendoring/OpenTelemetry.Instrumentation.ConfluentKafka/TracerProviderBuilderExtensions.Producer.cs |
Registers the producer activity source. |
src/Vendoring/OpenTelemetry.Instrumentation.ConfluentKafka/TracerProviderBuilderExtensions.Consumer.cs |
Registers the consumer activity source. |
src/Vendoring/OpenTelemetry.Instrumentation.ConfluentKafka/Shared/SemanticConventions.cs |
Updates semantic-convention constants. |
src/Vendoring/OpenTelemetry.Instrumentation.ConfluentKafka/Shared/Guard.cs |
Synchronizes validation helpers. |
src/Vendoring/OpenTelemetry.Instrumentation.ConfluentKafka/OpenTelemetryConsumeResultExtensions.cs |
Updates process-span behavior. |
src/Vendoring/OpenTelemetry.Instrumentation.ConfluentKafka/MeterProviderBuilderExtensions.Producer.cs |
Registers the producer meter. |
src/Vendoring/OpenTelemetry.Instrumentation.ConfluentKafka/MeterProviderBuilderExtensions.Consumer.cs |
Registers the consumer meter. |
src/Vendoring/OpenTelemetry.Instrumentation.ConfluentKafka/InstrumentedProducer.cs |
Updates producer telemetry. |
src/Vendoring/OpenTelemetry.Instrumentation.ConfluentKafka/InstrumentedConsumerBuilder.cs |
Synchronizes consumer construction. |
src/Vendoring/OpenTelemetry.Instrumentation.ConfluentKafka/InstrumentedConsumer.cs |
Updates consumer and exception telemetry. |
src/Vendoring/OpenTelemetry.Instrumentation.ConfluentKafka/ConfluentKafkaInstrumentedProducerBuilderOptions.cs |
Adds producer builder options. |
src/Vendoring/OpenTelemetry.Instrumentation.ConfluentKafka/ConfluentKafkaInstrumentedConsumerBuilderOptions.cs |
Adds consumer builder options. |
src/Vendoring/OpenTelemetry.Instrumentation.ConfluentKafka/ConfluentKafkaCommon.cs |
Defines instrumentation identity and instruments. |
src/Components/Telemetry.md |
Documents renamed Kafka metrics. |
src/Components/Aspire.Confluent.Kafka/ConfluentKafkaMetrics.cs |
Renames legacy metrics and client tag. |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 21 out of 21 changed files in this pull request and generated no new comments.
Suppressed comments (2)
src/Components/Aspire.Confluent.Kafka/ConfluentKafkaMetrics.cs:59
- These names do not match the underlying librdkafka statistics.
txmsgs/rxmsgscount messages transmitted to or consumed from brokers, while the v1.43 metrics represent producer send attempts and deliveries to the application. Queued or failed sends and prefetched-but-undelivered records therefore make this legacy meter disagree with the vendored meter for the same metric names. Keep the statistics under custom names, or emit these semantic-convention counters from the producer/consumer operation paths instead.
public const string TxMessages = "messaging.client.sent.messages";
public const string TxMessageBytes = "messaging.kafka.message.transmitted";
public const string RxMessages = "messaging.client.consumed.messages";
tests/Aspire.Confluent.Kafka.Tests/OtelInstrumentationTests.cs:15
- This new class can run in parallel with
OtelTracesTestsandOtelMetricsTests, although all three attach exporters to the same static KafkaActivitySource/Meter. Telemetry emitted here can consequently enter another test's exporter (and its unsynchronizedList), causing count assertions such asAssert.Equal(5, activities.Count)to fail nondeterministically. Put all Kafka telemetry tests in one serialized collection or otherwise disable parallel execution between them.
public class OtelInstrumentationTests
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
This comment has been minimized.
This comment has been minimized.
|
Retrying the failed CI jobs for this pull request from the CI run attempt. The rerun is being tracked in the rerun attempt. |
Adam Ratzman (adamint)
left a comment
There was a problem hiding this comment.
Could you take a look at these? I think they need to be fixed before merging.
|
FYI we shipped 0.3.0-alpha.1 last week. |
Local validation of Aspire PR #19319Latest update: 0.3.0-alpha.1Committed and pushed as
The 0.3 update adds cluster-ID enrichment and updates schema metadata without further metric renames relative to 0.2. The telemetry migration warning below still applies to the PR as a whole. Evidence for this update: Earlier validation: 0.2.0-alpha.2 plus review fixesResult: Functional and API compatibility validation passed. Telemetry backward compatibility does not hold: this PR intentionally replaces metric names, attributes, and span conventions. Tested state
Review fixes included
Validation results
The smoke application used public client-registration APIs and the repository's real Kafka broker fixture. It exercised both synchronous Observed metricsFor one topic, the application sent three messages using
The two 120 ms empty polls contributed about 0.243 seconds. Total recorded poll duration was about 1.292 seconds versus about 1.299 seconds measured around the calls. Cancellation did not add a successful poll measurement. A separate message deliberately failed value deserialization. Its consumer emitted one duration sample and one consumed-record count, both tagged Verified:
The legacy statistics counters and modern instrumentation counters retain their distinct meanings around deserialization failures; they should not be summed together without considering instrumentation scope. Backward compatibilityPublic registration APIs, settings, configuration schema, and connection-string handling are unchanged from the PR merge base. Package API compatibility passed against 13.4.6. Dashboard summary parsing and filtering accept both the old and new messaging-operation attributes, with the new attribute taking precedence. However, existing telemetry dashboards, alerts, and queries require migration:
Producer spans change from The If zero telemetry breaking changes are required, the PR does not meet that requirement. A compatibility mode preserving the old instruments/attributes would be needed, especially for the legacy opt-in meter. Otherwise, the documented migration must be accepted. Scope and evidence
|
Changes applied
|
FYI we shipped 0.3.0-alpha.2 this week, which contains some specific changes to this instrumentation. |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 908e2f89-e34c-451e-bb4e-d239dd7b566c
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 908e2f89-e34c-451e-bb4e-d239dd7b566c
Record duration for completed empty and EOF polls without counting messages or creating spans. Cover cancellation and consume failures, isolate trace capture by topic, and assert send and poll histograms independently. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Sync the tagged upstream source and shared helpers, preserving Aspire adaptations. Add cluster ID span enrichment and v1.44.0 schema metadata, with real-broker coverage and isolated consumer groups. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Preserve the Aspire 8 statistics schema instead of applying application-delivery metric names to broker statistics. Include the upstream error-status fix, regression coverage, and a user-facing breaking-change release note and migration guide. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
a14f1e1 to
3df5b6c
Compare
This comment has been minimized.
This comment has been minimized.
Leave detailed breaking-change and migration guidance to aspire.dev while retaining a concise README notice and accurate telemetry inventory. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
Active issues allow failed discovery continuations to evict newer cache entries and trigger unnecessary cluster discovery when tracing is disabled for producers and consumers.
2 open issues
1 resolved since last review
Review effort: Balanced
This comment has been minimized.
This comment has been minimized.
Start discovery only when a recorded span needs enrichment, retain tracing enablement after client construction, and respect tracing opt-out for message processing. Remove failed cache entries by key/task pair so stale waiters cannot evict newer lookups. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Tests selector15 / 101 PR test projects · 2 PR jobs, from 26 changed files. Selected PR test projects (15 / 101)
Selected PR jobs (2)
How these were chosen — grouped by what changed
🔧 🔧 📦 affected project 🔧 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 🧪 Job reasons
Selection computed for commit |
| { | ||
| var messagingSystem = OtlpHelpers.GetValue(span.Attributes, "messaging.system"); | ||
| var messagingOperation = OtlpHelpers.GetValue(span.Attributes, "messaging.operation"); | ||
| var messagingOperation = span.Attributes.GetValueWithFallback("messaging.operation.name", "messaging.operation"); |
There was a problem hiding this comment.
do we need this? shouldn't we just follow the standards?
| // Keep the Aspire 8 schema: librdkafka txmsgs/rxmsgs count broker traffic, | ||
| // not producer send attempts or messages delivered at the Consume boundary. |
There was a problem hiding this comment.
Is this the right thing to do?


Description
Updates the vendored
OpenTelemetry.Instrumentation.ConfluentKafkasource from0.1.0-alpha.2to the latest published version,0.3.0-alpha.4(upstream commit4cbaa1b137fc155a3f4ff62cfe034b78137a04eb). This adopts messaging semantic conventions v1.44.0, adds best-effort asynchronous cluster-ID enrichment, records consume-error telemetry, and includes the upstream fix that stops copying exception messages into span status descriptions.This is a source-vendoring update. Aspire's OpenTelemetry NuGet dependencies remain unchanged. The fork preserves internal visibility, explicit instrumentation identity/schema, AOT-compatible property fetching, and the local fix that records completed empty/EOF poll durations without message counts or spans. Local cluster-ID adaptations defer discovery until a recorded span needs enrichment and prevent failed lookups from evicting replacement cache entries.
Related issue: N/A (dependency refresh and review follow-up).
Release target and documentation
Rebased onto current
mainand retargeted fromrelease/17.0tomainfor Aspire 17.0, a release that accepts breaking changes. This is a breaking telemetry change, not a compatible servicing update. The stable Aspire integration's telemetry contract is not exempt merely because the upstream instrumentation is alpha. Public .NET registration APIs, settings, and connection-string handling are unchanged.The package README contains a brief 17.0 breaking-change notice. Detailed release notes and Kafka telemetry migration guidance belong on aspire.dev, maintained in microsoft/aspire.dev, rather than in a separate migration document in this repository. The vendoring README and internal telemetry inventory remain accurate. No companion aspire.dev documentation PR is linked yet.
Breaking telemetry changes
The default
OpenTelemetry.Instrumentation.ConfluentKafkameter replacesmessaging.publish.durationandmessaging.receive.durationwithmessaging.client.operation.duration; filtermessaging.operation.name=sendorpollto retain separate latency distributions. Message counters becomemessaging.client.sent.messagesandmessaging.client.consumed.messages.Attributes change to
messaging.client.id,messaging.operation.name/.type,messaging.consumer.group.name, stringmessaging.destination.partition.id, andmessaging.kafka.offset. Producer spans becomesend <topic>, receive spans becomepoll <topic>withClientkind, and processing spans becomeprocess <topic>. Detailed mappings and query guidance should be included in the companion aspire.dev documentation.The legacy
Aspire.Confluent.Kafkastatistics meter keeps its existing schema, includingmessaging.publish.messages,messaging.receive.messages,messaging.client_id, tags, and units. Its librdkafka broker-traffic statistics must not be relabeled as modern send-attempt/application-delivery counters or summed with them.EnableAspire8ConfluentKafkaMetricsenables this additional meter; it does not restore the old default duration histograms or span schema.Latest upstream releases
0.3.0-alpha.4(2026-Oct-21): upstream OpenTelemetry core dependency update to1.20.1.0.3.0-alpha.3(2026-Oct-15): upstream OpenTelemetry core dependency update to1.20.0.0.3.0-alpha.2(2026-Sep-18): removes unsanitized exception messages from Kafka span status descriptions; upstream OpenTelemetry core dependency update to1.19.0.0.3.0-alpha.1: messaging conventions v1.44.0 andmessaging.kafka.cluster.idspan enrichment.Only vendored source changes are adopted; the upstream package's dependency version bumps are not applied globally. Full upstream changelog.
Review follow-up
error.typeremain, exceptions propagate, and status descriptions are empty.Validation
Validated the Kafka review fixes at commit
2afc67496406616f7b88c1eedeeb57a1315affc8on macOS ARM64, based onmaincommit318bf5294a3402a58de23ea0468dfc790d47f4fe. These results supersede the older .NET 8/9/10 reports; current main targets .NET 10/11. The targeted cache/discovery regressions failed in all 16 cases before the fixes; the expanded instrumentation class passes all 46 cases afterward.Kafka: passed, 370 tests, 12 existing unsupported-tracing skips, zero failures across
net10.0andnet11.0. Includes real Docker brokers, keyed/unkeyed registrations, exact message-counter values and units, separate send/poll histogram samples, cluster IDs, schema metadata, legacy statistics, empty/EOF polls, cancellation, errors, stale cache waiters, and deferred discovery for both clients and all producer/consumer overloads.Dashboard: passed, 11 tests, zero failures at commit
3df5b6ccf221d8879163c2b12902aeafe9a28b9a, including modern/legacy messaging summaries and SQLite text filtering. No Dashboard code has changed in subsequent commits, so these tests were not rerun for the cluster-discovery fixes. Restore emits the existingNU1903advisory forMicrosoft.DotNet.ILCompiler11.0.0-rc.2.26504.105; this PR does not change that dependency.Package/API compatibility: passed against baseline
13.6.2, zero warnings/errors, rerun after the cluster-discovery fixes. Forced uncached validation during Pack; APICompat reported no breaking .NET APIs. Current main's existing exclusions ignore baselinenet8.0/net9.0assets; this PR does not change framework support.Compared every included vendor source with the exact upstream release and confirmed the documented adaptations; shared
Guard.csandSemanticConventions.csmatch exactly. Windows/Linux runs and browser-level Dashboard rendering were not performed.Checklist
<remarks />and<code />elements on your triple slash comments?