fixing traces #3082

Merged
mfreeman451 merged 2 commits from refs/pull/3082/head into staging 2026-03-26 00:37:44 +00:00
mfreeman451 commented 2026-03-26 00:32:50 +00:00 (Migrated from github.com)
Owner

Imported from GitHub pull request.

Original GitHub pull request: #3083
Original author: @mfreeman451
Original URL: https://github.com/carverauto/serviceradar/pull/3083
Original created: 2026-03-26T00:32:50Z
Original updated: 2026-03-26T00:37:45Z
Original head: carverauto/serviceradar:updates/traces-missing
Original base: staging
Original merged: 2026-03-26T00:37:44Z by @mfreeman451

IMPORTANT: Please sign the Developer Certificate of Origin

Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include
a DCO sign-off statement indicating the DCO acceptance in one commit message. Here
is an example DCO Signed-off-by line in a commit message:

Signed-off-by: J. Doe <j.doe@domain.com>

Describe your changes

Code checklist before requesting a review

  • I have signed the DCO?
  • The build completes without errors?
  • All tests are passing when running make test?
Imported from GitHub pull request. Original GitHub pull request: #3083 Original author: @mfreeman451 Original URL: https://github.com/carverauto/serviceradar/pull/3083 Original created: 2026-03-26T00:32:50Z Original updated: 2026-03-26T00:37:45Z Original head: carverauto/serviceradar:updates/traces-missing Original base: staging Original merged: 2026-03-26T00:37:44Z by @mfreeman451 --- ## IMPORTANT: Please sign the Developer Certificate of Origin Thank you for your contribution to ServiceRadar. Please note, when contributing, the developer must include a [DCO sign-off statement]( https://developercertificate.org/) indicating the DCO acceptance in one commit message. Here is an example DCO Signed-off-by line in a commit message: ``` Signed-off-by: J. Doe <j.doe@domain.com> ``` ## Describe your changes ## Issue ticket number and link ## Code checklist before requesting a review - [ ] I have signed the DCO? - [ ] The build completes without errors? - [ ] All tests are passing when running make test?
Copilot commented 2026-03-26 00:36:34 +00:00 (Migrated from github.com)
Author
Owner

Imported GitHub PR review comment.

Original author: @Copilot
Original URL: https://github.com/carverauto/serviceradar/pull/3083#discussion_r2991836370
Original created: 2026-03-26T00:36:34Z
Original path: go/pkg/consumers/db-event-writer/service_test.go
Original line: 90

Test name is now misleading: the assertion includes trace subject expansion ("otel.traces" added when configuring "otel.traces.raw"), not just the derived metrics filter. Consider renaming the test (or splitting into separate tests) so failures point to the correct behavior under test.

Imported GitHub PR review comment. Original author: @Copilot Original URL: https://github.com/carverauto/serviceradar/pull/3083#discussion_r2991836370 Original created: 2026-03-26T00:36:34Z Original path: go/pkg/consumers/db-event-writer/service_test.go Original line: 90 --- Test name is now misleading: the assertion includes trace subject expansion ("otel.traces" added when configuring "otel.traces.raw"), not just the derived metrics filter. Consider renaming the test (or splitting into separate tests) so failures point to the correct behavior under test.
Copilot commented 2026-03-26 00:36:35 +00:00 (Migrated from github.com)
Author
Owner

Imported GitHub PR review comment.

Original author: @Copilot
Original URL: https://github.com/carverauto/serviceradar/pull/3083#discussion_r2991836397
Original created: 2026-03-26T00:36:35Z
Original path: go/pkg/consumers/db-event-writer/service.go
Original line: 418

consumerFilterSubjects currently returns both "otel.traces" and "otel.traces.raw" even when the configured subject is "otel.traces.raw". In multi-stream mode, the Processor's routing only matches configured stream subjects (e.g., "otel.traces.raw"), so any legacy "otel.traces" messages delivered by the widened consumer filter won't map to a table and can cause ProcessBatch to error (leaving some messages in the batch unacked and repeatedly redelivered). Consider only expanding legacy->raw (i.e., return both when config is "otel.traces"), or ensure the stream routing config/logic also handles "otel.traces" when subscribing to it (e.g., add a stream entry or normalize subjects consistently between consumer filters and routing).

	case "otel.traces":
		// The collector publishes traces under `otel.traces.raw`.
		// Preserve legacy config compatibility while widening the JetStream
		// consumer filter so trace messages are actually delivered.
		return []string{"otel.traces", "otel.traces.raw"}
	case "otel.traces.raw":
		// When the configured subject already matches the collector's publish
		// subject, don't subscribe to the legacy subject to avoid delivering
		// messages that won't be routed by processors configured only for
		// `otel.traces.raw`.
		return []string{"otel.traces.raw"}
Imported GitHub PR review comment. Original author: @Copilot Original URL: https://github.com/carverauto/serviceradar/pull/3083#discussion_r2991836397 Original created: 2026-03-26T00:36:35Z Original path: go/pkg/consumers/db-event-writer/service.go Original line: 418 --- consumerFilterSubjects currently returns both "otel.traces" and "otel.traces.raw" even when the configured subject is "otel.traces.raw". In multi-stream mode, the Processor's routing only matches configured stream subjects (e.g., "otel.traces.raw"), so any legacy "otel.traces" messages delivered by the widened consumer filter won't map to a table and can cause ProcessBatch to error (leaving some messages in the batch unacked and repeatedly redelivered). Consider only expanding legacy->raw (i.e., return both when config is "otel.traces"), or ensure the stream routing config/logic also handles "otel.traces" when subscribing to it (e.g., add a stream entry or normalize subjects consistently between consumer filters and routing). ```suggestion case "otel.traces": // The collector publishes traces under `otel.traces.raw`. // Preserve legacy config compatibility while widening the JetStream // consumer filter so trace messages are actually delivered. return []string{"otel.traces", "otel.traces.raw"} case "otel.traces.raw": // When the configured subject already matches the collector's publish // subject, don't subscribe to the legacy subject to avoid delivering // messages that won't be routed by processors configured only for // `otel.traces.raw`. return []string{"otel.traces.raw"} ```
Copilot commented 2026-03-26 00:36:35 +00:00 (Migrated from github.com)
Author
Owner

Imported GitHub PR review comment.

Original author: @Copilot
Original URL: https://github.com/carverauto/serviceradar/pull/3083#discussion_r2991836427
Original created: 2026-03-26T00:36:35Z
Original path: go/pkg/consumers/db-event-writer/service_test.go
Original line: 89

This assertion expects the service to subscribe to both "otel.traces" and "otel.traces.raw" even though the configured stream subject is only "otel.traces.raw". If the processor/routing config doesn't also handle "otel.traces", receiving legacy trace messages can trigger batch-processing errors and leave other messages unacked. Align the expected subjects here with the intended routing behavior (either subscribe only to "otel.traces.raw" when configured that way, or ensure routing supports both).

		[]string{"logs.otel.processed", "otel.metrics", "otel.metrics.>", "otel.traces.raw"},
Imported GitHub PR review comment. Original author: @Copilot Original URL: https://github.com/carverauto/serviceradar/pull/3083#discussion_r2991836427 Original created: 2026-03-26T00:36:35Z Original path: go/pkg/consumers/db-event-writer/service_test.go Original line: 89 --- This assertion expects the service to subscribe to both "otel.traces" and "otel.traces.raw" even though the configured stream subject is only "otel.traces.raw". If the processor/routing config doesn't also handle "otel.traces", receiving legacy trace messages can trigger batch-processing errors and leave other messages unacked. Align the expected subjects here with the intended routing behavior (either subscribe only to "otel.traces.raw" when configured that way, or ensure routing supports both). ```suggestion []string{"logs.otel.processed", "otel.metrics", "otel.metrics.>", "otel.traces.raw"}, ```
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
carverauto/serviceradar!3082
No description provided.