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

Add missing otel attributes to Spans exported to Cloud Trace #365

Merged
merged 9 commits into from
Apr 26, 2022

Conversation

jsuereth
Copy link
Contributor

@jsuereth jsuereth commented Apr 22, 2022

Fixes #332

Turns out we never had a "raw" Span -> Span translation test before now. Imported a few testing libs to make this a thing, and may expand on unit testing other span conversion aspects.

  • Creates new "generic" monitored resource mapping internal library
  • Modify Collector exporter to leverage this module with adapters.
  • Leverage monitored resource mapping library to annotate trace spans with monitored resource labels.
  • Add InstrumnetationLibrary labels to span attributes.

@jsuereth jsuereth marked this pull request as ready for review April 25, 2022 15:34
Copy link
Contributor

@dashpole dashpole left a comment

Choose a reason for hiding this comment

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

lgtm after import is fixed.

exporter/collector/integrationtest/go.mod Outdated Show resolved Hide resolved
@@ -31,8 +33,11 @@ require (
github.com/googleapis/gax-go/v2 v2.2.0 // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
go.opencensus.io v0.23.0 // indirect
go.opentelemetry.io/collector/model v0.49.0 // indirect
Copy link
Contributor

Choose a reason for hiding this comment

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

This is somewhat unfortunate... The collector and the go SDK have two different packages for semconv. E.g. https://github.com/open-telemetry/opentelemetry-go/tree/main/semconv/v1.9.0 vs https://github.com/open-telemetry/opentelemetry-collector/tree/main/semconv/v1.9.0. I think we should probably use go.opentelemetry.io/otel/semconv/v1.9.0, rather than the collector one. It seems less strange for the collector to depend on the SDK than for the SDK exporter to depend on the collector.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good point. I didn't even realize I was pulling the wrong one....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I'm having trouble finding a published version from the SDK -

https://pkg.go.dev/go.opentelemetry.io/otel/semconv/v1.9.0?tab=versions

$ go get go.opentelemetry.io/otel/semconv/v1.9.0
go get: module go.opentelemetry.io/otel@upgrade found (v1.6.3), but does not contain package go.opentelemetry.io/otel/semconv/v1.9.0

Should I move back to a version that exists? How often does go publish these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Migrated to v1.7.0 (latest released version) of semconv.

internal/resourcemapping/resourcemapping.go Outdated Show resolved Hide resolved
internal/resourcemapping/resourcemapping.go Show resolved Hide resolved
@jsuereth jsuereth merged commit f4bd197 into main Apr 26, 2022
@jsuereth jsuereth deleted the wip-span-attribute-expansion branch April 26, 2022 15:24
damemi added a commit that referenced this pull request Apr 29, 2022
* Do not use IsZero on timestamps obtained from pdata (#336)

* do not use IsZero on timestamps obtained from pdata

* send summary counts as doubles instead of integers (#338)

* drop data points with no value set (#339)

* handle NaN sums in histograms and exponential histograms (#340)

* Fix broken links, typos in docs (#345)

* Update fixtures based on upstream changes to the prometheus receiver (#346)

* update fixtures based on upstream changes to the prometheus receiver

* feat(propagator): create a new one-way propagator (#343)

* feat(propagator): create a new one-way propagator

CloudTraceOneWayPropagator moves trace info from X-cloud-trace-context
into 'traceparent'.

* Apply suggestions from code review

Co-authored-by: David Ashpole <dashpole@google.com>

* Update propagator to extract both, emit only traceparent

Users will no longer need to create a composite propagator, and there's
a clear precedence order between trace headers.

Co-authored-by: David Ashpole <dashpole@google.com>

* Demonstrate that integration tests aren't catching failures (#349)

* demonstrate that integration tests aren't catching failures

* Update dependencies (#347)

update dependencies

* address remaining nits from #347 (#351)

* prepare for release (#353)

* add integration test with metrics from the prometheus receiver (#348)

* add make commands for release (#354)

* Send timeseries synchronously (#358)

* send timeseries syncronously

* update docs, and fix existing broken test

* Drop histograms without a sum (#359)

* update opentelemetry collector to v0.49.0

* drop histograms without a sum

* Add sum of squared deviation estimate (#341)

* add sum of squared deviation estimate

* Add missing otel attributes to Spans exported to Cloud Trace (#365)

* Add instrumentation library to span attributes.

* Add test to ensure resource + instrumentation-scope labels show up in spans.

* Expand testing.

* Move resource mapping into shared module, and add monitored resource labels to trace spans.

* Do the go mod tidy shuffle.

* Fix docker build to include new shared module.

* Bump back to go 1.17

* Migrate to SDK version of semantic convention constants instead of Collector's.

* Clean up documents.

* Normalize histograms, summaries, and exponential histograms (#360)

* normalize remaining cumulative data points

* use the timeout passed to the metrics exporter (#369)

* make gotidy

* Resolve conflicts from missed file

Co-authored-by: David Ashpole <dashpole@google.com>
Co-authored-by: Marc Dougherty <muncus@users.noreply.github.com>
Co-authored-by: Josh Suereth <joshuasuereth@google.com>
damemi added a commit that referenced this pull request Apr 29, 2022
* Do not use IsZero on timestamps obtained from pdata (#336)

* do not use IsZero on timestamps obtained from pdata

* send summary counts as doubles instead of integers (#338)

* drop data points with no value set (#339)

* handle NaN sums in histograms and exponential histograms (#340)

* Fix broken links, typos in docs (#345)

* Update fixtures based on upstream changes to the prometheus receiver (#346)

* update fixtures based on upstream changes to the prometheus receiver

* feat(propagator): create a new one-way propagator (#343)

* feat(propagator): create a new one-way propagator

CloudTraceOneWayPropagator moves trace info from X-cloud-trace-context
into 'traceparent'.

* Apply suggestions from code review

Co-authored-by: David Ashpole <dashpole@google.com>

* Update propagator to extract both, emit only traceparent

Users will no longer need to create a composite propagator, and there's
a clear precedence order between trace headers.

Co-authored-by: David Ashpole <dashpole@google.com>

* Demonstrate that integration tests aren't catching failures (#349)

* demonstrate that integration tests aren't catching failures

* Update dependencies (#347)

update dependencies

* address remaining nits from #347 (#351)

* prepare for release (#353)

* add integration test with metrics from the prometheus receiver (#348)

* add make commands for release (#354)

* Send timeseries synchronously (#358)

* send timeseries syncronously

* update docs, and fix existing broken test

* Drop histograms without a sum (#359)

* update opentelemetry collector to v0.49.0

* drop histograms without a sum

* Add sum of squared deviation estimate (#341)

* add sum of squared deviation estimate

* Add missing otel attributes to Spans exported to Cloud Trace (#365)

* Add instrumentation library to span attributes.

* Add test to ensure resource + instrumentation-scope labels show up in spans.

* Expand testing.

* Move resource mapping into shared module, and add monitored resource labels to trace spans.

* Do the go mod tidy shuffle.

* Fix docker build to include new shared module.

* Bump back to go 1.17

* Migrate to SDK version of semantic convention constants instead of Collector's.

* Clean up documents.

* Normalize histograms, summaries, and exponential histograms (#360)

* normalize remaining cumulative data points

* use the timeout passed to the metrics exporter (#369)

Co-authored-by: David Ashpole <dashpole@google.com>
Co-authored-by: Marc Dougherty <muncus@users.noreply.github.com>
Co-authored-by: Josh Suereth <joshuasuereth@google.com>
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.

Add span attributes for library name and version
2 participants