Skip to content

fix(go): drop invalid enum values instead of recording empty attributes - #47

Merged
yordis merged 2 commits into
mainfrom
yordis/test-go-consumer-harness
Oct 13, 2026
Merged

yordis merged 2 commits into
mainfrom
yordis/test-go-consumer-harness

Conversation

@yordis

@yordis yordis commented Oct 13, 2026 •

Copy link
Copy Markdown
Member
  • Go output was only checked for building, so runtime mistakes and documentation examples that no longer compile slipped through review.
  • Misuse the type system is meant to reject had no proof it stays rejected as the templates evolve.
  • The zero value of an enum is the one invalid value Go cannot forbid, and it silently recorded an empty attribute.
  • Invalid attributes are dropped without reporting to match the OpenTelemetry Go SDK, which reserves the global error handler for irremediable errors.

@cursor

cursor Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes generated telemetry attribute semantics for enum zero values and touches all signal templates, so downstream generated code behavior shifts silently unless regenerated; test harness expansion reduces regression risk.

Overview
Generated Go enum attributes now carry a private ok flag so only declared members are valid; KeyValue() on a zero enum returns an invalid attribute.KeyValue, and metrics, spans, events, and entities skip any typed attribute whose KeyValue() is not valid (imported/raw attributes are unchanged).

Docs and CI: The Go template README describes this behavior. check_go_output.sh copies consumer/ tests into the build, runs go test, and adds compile-fail/ cases that must fail to build with expected compiler errors.

Tests: Consumer tests cover runtime recording for attributes, entities, events, metrics, and spans; compile-fail fixtures lock in that enums cannot be constructed from outside the package and that API misuse stays a compile error.

Reviewed by Cursor Bugbot for commit c0d30fc. Bugbot is set up for automated code reviews on this repo. Configure here.

@yordis yordis changed the title yordis/test go consumer harness fix(go): report invalid enum values instead of recording empty attributes Oct 13, 2026
@coderabbitai

coderabbitai Bot commented Oct 13, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 44 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 81b4eee7-ff54-4d0d-9b5c-a71cf0d68194
📥 Commits

Reviewing files that changed from the base of the PR and between 039cf30 and c0d30fc.

📒 Files selected for processing: 53
  • buildscripts/check_go_output.sh
  • templates/code/go/README.md
  • templates/code/go/attributes.go.j2
  • templates/code/go/entities.go.j2
  • templates/code/go/events.go.j2
  • templates/code/go/metrics.go.j2
  • templates/code/go/spans.go.j2
  • templates/code/go/tests/attributes/compile-fail/enum_from_outside_package.expected-error.txt
  • templates/code/go/tests/attributes/compile-fail/enum_from_outside_package.go
  • templates/code/go/tests/attributes/consumer/myappattr/attributes_test.go
  • templates/code/go/tests/attributes/expected/authattr/attributes.go
  • templates/code/go/tests/attributes/expected/myappattr/attributes.go
  • templates/code/go/tests/entities/consumer/myappentity/entities_test.go
  • templates/code/go/tests/entities/expected/myappattr/attributes.go
  • templates/code/go/tests/entities/expected/myappentity/entities.go
  • templates/code/go/tests/events/consumer/myappevent/events_test.go
  • templates/code/go/tests/events/expected/myappattr/attributes.go
  • templates/code/go/tests/events/expected/myappevent/events.go
  • templates/code/go/tests/events/expected/workerevent/events.go
  • templates/code/go/tests/histogram_boundaries_by_unit/expected/myappmetric/metrics.go
  • templates/code/go/tests/histogram_boundaries_by_unit_params/expected/myappmetric/metrics.go
  • templates/code/go/tests/imported/consumer/myappmetric/metrics_test.go
  • templates/code/go/tests/imported/expected/edgeevent/events.go
  • templates/code/go/tests/imported/expected/edgemetric/metrics.go
  • templates/code/go/tests/imported/expected/edgespan/spans.go
  • templates/code/go/tests/imported/expected/myappmetric/metrics.go
  • templates/code/go/tests/imported_typed/expected/myappevent/events.go
  • templates/code/go/tests/imported_typed/expected/myappmetric/metrics.go
  • templates/code/go/tests/imported_typed/expected/myappspan/spans.go
  • templates/code/go/tests/metrics/compile-fail/wrong_option_type.expected-error.txt
  • templates/code/go/tests/metrics/compile-fail/wrong_option_type.go
  • templates/code/go/tests/metrics/consumer/myappmetric/metrics_test.go
  • templates/code/go/tests/metrics/expected/authattr/attributes.go
  • templates/code/go/tests/metrics/expected/authmetric/metrics.go
  • templates/code/go/tests/metrics/expected/myappattr/attributes.go
  • templates/code/go/tests/metrics/expected/myappmetric/metrics.go
  • templates/code/go/tests/metrics/expected/workermetric/metrics.go
  • templates/code/go/tests/no_namespace/consumer/heartbeatspan/spans_test.go
  • templates/code/go/tests/no_namespace/expected/exceptionevent/events.go
  • templates/code/go/tests/no_namespace/expected/heartbeatspan/spans.go
  • templates/code/go/tests/no_namespace/expected/hostentity/entities.go
  • templates/code/go/tests/params/expected/billingmetric/metrics.go
  • templates/code/go/tests/root_package_from_import_path/expected/myappmetric/metrics.go
  • templates/code/go/tests/root_package_version_segment/expected/myappmetric/metrics.go
  • templates/code/go/tests/spans/compile-fail/raw_string_for_typed_attribute.expected-error.txt
  • templates/code/go/tests/spans/compile-fail/raw_string_for_typed_attribute.go
  • templates/code/go/tests/spans/compile-fail/sealed_option_interface.expected-error.txt
  • templates/code/go/tests/spans/compile-fail/sealed_option_interface.go
  • templates/code/go/tests/spans/consumer/myappspan/spans_test.go
  • templates/code/go/tests/spans/expected/myappattr/attributes.go
  • templates/code/go/tests/spans/expected/myappspan/spans.go
  • templates/code/go/tests/spans/expected/workerspan/spans.go
  • templates/code/go/weaver.yaml
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 5a71c24. Configure here.

Comment thread buildscripts/check_go_output.sh
Comment thread templates/code/go/spans.go.j2
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis yordis changed the title fix(go): report invalid enum values instead of recording empty attributes fix(go): drop invalid enum values instead of recording empty attributes Oct 13, 2026
@yordis
yordis force-pushed the yordis/test-go-consumer-harness branch from 5a71c24 to d2ad8d4 Compare October 13, 2026 20:59
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
@yordis
yordis merged commit 47bbcf2 into main Oct 13, 2026
7 checks passed
@yordis
yordis deleted the yordis/test-go-consumer-harness branch October 13, 2026 21:24
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.

1 participant