Skip to content

fix(otel): preserve configured service name - #593

Merged
zhongkechen merged 3 commits into
mainfrom
codex/preserve-otel-service-name
Aug 5, 2026
Merged

fix(otel): preserve configured service name#593
zhongkechen merged 3 commits into
mainfrom
codex/preserve-otel-service-name

Conversation

@zhongkechen

@zhongkechen zhongkechen commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

  • stop ExecutionOtelPlugin from overriding the caller-configured OpenTelemetry resource service.name
  • keep service identity owned by application/provider configuration, matching InvocationOtelPlugin, Python, and TypeScript
  • verify workflow and invocation spans retain the configured resource service name

Testing

  • mvn -pl otel-plugin test (145 tests)
  • mvn -pl otel-plugin spotless:apply
  • git diff --check

Related change

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen temporarily deployed to ai-pr-review-runtime August 4, 2026 23:46 — with GitHub Actions Inactive
@zhongkechen
zhongkechen temporarily deployed to ai-pr-review-runtime August 4, 2026 23:46 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen marked this pull request as ready for review August 4, 2026 23:51
@zhongkechen
zhongkechen requested a review from a team August 4, 2026 23:51
@zhongkechen
zhongkechen temporarily deployed to ai-pr-review-runtime August 4, 2026 23:51 — with GitHub Actions Inactive
@zhongkechen
zhongkechen temporarily deployed to ai-pr-review-runtime August 4, 2026 23:51 — with GitHub Actions Inactive
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@zhongkechen
zhongkechen deployed to ai-pr-review-runtime August 5, 2026 17:25 — with GitHub Actions Active
@zhongkechen
zhongkechen temporarily deployed to ai-pr-review-runtime August 5, 2026 17:25 — with GitHub Actions Inactive
@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codex AI review

No actionable findings.

Residual risk: read-only review; the updated OpenTelemetry tests were not executed.

Reviewed commit e8fad6e62eec2420f1db11698ca83b42fa6b2c98. Workflow run

@github-actions

github-actions Bot commented Aug 5, 2026

Copy link
Copy Markdown

Claude AI review

Review

Verdict: No actionable findings — the change is correct and adequately tested.

The PR removes the constructor code in ExecutionOtelPlugin that force-overrode the caller-configured OTel resource service.name with the constant "workflow", plus the now-unused imports and the SERVICE_NAME constant.

Checks performed:

  • The removed tracerProviderBuilder.addResource(...) merged a service.name="workflow" resource into the caller's; OTel addResource merge semantics give the added resource precedence on key collision, so it did override the caller's value. Removing it correctly hands service identity back to application/provider config, matching InvocationOtelPlugin (no resource manipulation) and the claimed Python/TS parity.
  • All references to the removed symbols (Attributes, Resource, ServiceAttributes, SERVICE_NAME) are gone from main source — no dangling imports or unused-symbol issues (ExecutionOtelPlugin.java:23,90,163-165 fully removed).
  • The renamed test spans_preserveConfiguredServiceName (ExecutionOtelPluginTest.java) configures a resource via setResource(...) and asserts every finished span (Workflow + Invocation, since it drives a terminal invocation) preserves the configured name — a valid regression guard.
  • No other test/cloud assertion still expects service.name="workflow"; the cloud test filters by unique function name and concerns only the "invocation" service.

No correctness, generics/typing, serialization, replay, suspension/retry, concurrency, or lifecycle regressions were introduced.

Residual note (non-blocking, unchanged lines)

This PR removes the only production usage of io.opentelemetry.semconv.ServiceAttributes, leaving the opentelemetry-semconv dependency in otel-plugin/pom.xml:58-63 orphaned. Not needed for the fix, but a reasonable follow-up cleanup.

Test risk

Low. The unit test covers both exported span types for the preserved name. Residual: there is no negative-path assertion confirming a default/unset service name when the caller configures none, and no integration/replay-level test asserting service name preservation across suspend/resume invocations — minor, given the unit coverage.

Reviewed commit e8fad6e62eec2420f1db11698ca83b42fa6b2c98. Workflow run

@zhongkechen
zhongkechen merged commit 33f50fa into main Aug 5, 2026
9 checks passed
@zhongkechen
zhongkechen deleted the codex/preserve-otel-service-name branch August 5, 2026 19:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants