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

Minor changes to Obsreport #942

Merged

Conversation

james-bebbington
Copy link
Member

Link to tracking Issue:
#918

Description:
Made the "transport" parameter that's used to set a corresponding attribute value optional for obsreport receiver functions. This parameter doesn't make sense for all receivers.

Also added a comment regarding using the internal data structure with the CountMetricPoints function.

@james-bebbington james-bebbington changed the title Made the "transport" parameter optional for obsreport receiver functi… Minor changes to Obsreport May 10, 2020
tag.Upsert(tagKeyTransport, transport, tag.WithTTL(tag.TTLNoPropagation)),
mutators := make([]tag.Mutator, 0, 2)
mutators = append(mutators, tag.Upsert(tagKeyReceiver, receiver, tag.WithTTL(tag.TTLNoPropagation)))
if transport != "" {
Copy link
Member Author

Choose a reason for hiding this comment

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

Not totally sure about using the empty string to signify an optional parameter, but it seems fairly idiomatic from what I can tell, and has the advantage here of being a non-breaking change.

@codecov-io
Copy link

codecov-io commented May 10, 2020

Codecov Report

Merging #942 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #942   +/-   ##
=======================================
  Coverage   85.52%   85.52%           
=======================================
  Files         184      184           
  Lines       13545    13546    +1     
=======================================
+ Hits        11584    11585    +1     
  Misses       1498     1498           
  Partials      463      463           
Impacted Files Coverage Δ
obsreport/obsreport.go 94.44% <ø> (ø)
obsreport/obsreport_receiver.go 100.00% <100.00%> (ø)
translator/internaldata/resource_to_oc.go 73.52% <0.00%> (-2.95%) ⬇️
receiver/opencensusreceiver/octrace/opencensus.go 93.33% <0.00%> (+3.33%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 80ee563...8f1505f. Read the comment docs.

…ons and added a note about MetricCount when using the new internal data structure
@bogdandrutu bogdandrutu merged commit 8711190 into open-telemetry:master May 11, 2020
wyTrivail pushed a commit to mxiamxia/opentelemetry-collector that referenced this pull request Jul 13, 2020
…ons and added a note about MetricCount when using the new internal data structure (open-telemetry#942)
MovieStoreGuy pushed a commit to atlassian-forks/opentelemetry-collector that referenced this pull request Nov 11, 2021
…obuf bindings locally (open-telemetry#942)

* Import open-telemetry/opentelemetry-proto submodule under internal

* Commit changes in updated/new protobuf files

* Refer to new location of .pb.go files after rewrite from import

* Describe in CHANGELOG
Troels51 pushed a commit to Troels51/opentelemetry-collector that referenced this pull request Jul 5, 2024
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.

4 participants