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

Fix OTLP waitForReady, not set from config #1254

Merged
merged 1 commit into from
Jul 7, 2020

Conversation

bogdandrutu
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Jul 2, 2020

Codecov Report

Merging #1254 into master will decrease coverage by 0.03%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1254      +/-   ##
==========================================
- Coverage   88.54%   88.50%   -0.04%     
==========================================
  Files         207      207              
  Lines       14945    14948       +3     
==========================================
- Hits        13233    13230       -3     
- Misses       1283     1286       +3     
- Partials      429      432       +3     
Impacted Files Coverage Δ
exporter/otlpexporter/exporter.go 71.87% <88.88%> (+0.90%) ⬆️
translator/internaldata/resource_to_oc.go 86.04% <0.00%> (-4.66%) ⬇️
receiver/otlpreceiver/otlp.go 90.47% <0.00%> (-3.18%) ⬇️

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 c78701f...11322bc. Read the comment docs.

gs.logExporter = otlplogcol.NewLogServiceClient(gs.grpcClientConn)
gs.metadata = metadata.New(config.GRPCClientSettings.Headers)

gs := &grpcSender{
Copy link
Member

Choose a reason for hiding this comment

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

nit: You don't need the gs variable anymore, can just return inline.

Copy link
Member Author

Choose a reason for hiding this comment

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

I had that before, but I kind of don't like that the error nil is most invisible.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also a decent compiler should inline that so not worried that much

@bogdandrutu bogdandrutu merged commit 4783107 into open-telemetry:master Jul 7, 2020
@bogdandrutu bogdandrutu deleted the fixotlp branch July 7, 2020 23:54
wyTrivail pushed a commit to mxiamxia/opentelemetry-collector that referenced this pull request Jul 13, 2020
MovieStoreGuy pushed a commit to atlassian-forks/opentelemetry-collector that referenced this pull request Nov 11, 2021
* Update Span API event methods

Remove the context argument from the event methods. It is unused and can
be added back in as a passed option if needed in the future.

Update AddEvent to accept a required name and a set of options. These
options are the new EventOption type that can be used to configure a
SpanConfig Timestamp and Attributes.

Remove the AddEventWithTimestamp method as it is redundant to calling
AddEvent with a WithTimestamp option.

Update RecordError to also accept the EventOptions.

* Add changes to CHANGELOG

* Add LifeCycleOption

Use the LifeCycleOption to encapsulate the options passed to a span for
life cycle events.
hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023
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