Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cmd/telemetrygen] Change span kind from an attribute to top level info #24303

Merged

Conversation

edwintye
Copy link
Contributor

Description: Remove the span attribute span.kind and instead set the Kind (which is a top level span information) directly using trace.WithSpanKind.

Link to tracking Issue: #24286

Testing: Added a rudimentary test to ensure that Kind is not Internal which is the default when not specified.

Documentation: None. This is probably the original intention.

@edwintye edwintye requested a review from a team July 17, 2023 18:06
@github-actions github-actions bot added the cmd/telemetrygen telemetrygen command label Jul 17, 2023
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Thanks for opening this, one change needed

cmd/telemetrygen/internal/traces/worker.go Outdated Show resolved Hide resolved
@edwintye edwintye force-pushed the issue-24286-span-kind-attribute branch 2 times, most recently from cb7b1de to aab0ec3 Compare July 19, 2023 10:13
Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

Please add a changelog and we can get this merged 👍🏻

@edwintye edwintye force-pushed the issue-24286-span-kind-attribute branch 2 times, most recently from 43edfcb to 839d963 Compare July 20, 2023 22:31
@edwintye
Copy link
Contributor Author

Added a chlog entry and rebased, looks like a couple tests fails but seems unrelated to my untrained eyes.

@mx-psi
Copy link
Member

mx-psi commented Jul 21, 2023

@edwintye this one does need addressing on this PR:

telemetrygen/internal/traces/worker_test.go:8: File is not `gci`-ed with --skip-generated -s standard -s default -s prefix(github.com/open-telemetry/opentelemetry-collector-contrib) (gci)
	"go.opentelemetry.io/otel/trace"
telemetrygen/internal/traces/worker_test.go:15: File is not `gci`-ed with --skip-generated -s standard -s default -s prefix(github.com/open-telemetry/opentelemetry-collector-contrib) (gci)
	sdktrace "go.opentelemetry.io/otel/sdk/trace"
telemetrygen/internal/traces/worker_test.go:11: File is not `goimports`-ed with -local github.com/open-telemetry/opentelemetry-collector-contrib (goimports)

@edwintye edwintye force-pushed the issue-24286-span-kind-attribute branch from 839d963 to 0adf3df Compare July 21, 2023 09:21
@edwintye edwintye force-pushed the issue-24286-span-kind-attribute branch from 0adf3df to de64d80 Compare July 21, 2023 09:22
@songy23 songy23 added the ready to merge Code review completed; ready to merge by maintainers label Jul 21, 2023
@jpkrohling jpkrohling merged commit 2607ed8 into open-telemetry:main Jul 21, 2023
@github-actions github-actions bot added this to the next release milestone Jul 21, 2023
@edwintye edwintye deleted the issue-24286-span-kind-attribute branch July 21, 2023 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cmd/telemetrygen telemetrygen command ready to merge Code review completed; ready to merge by maintainers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants