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 Retry and Queue settings to honeycomb exporter #2714

Merged
merged 1 commit into from
Apr 12, 2021

Conversation

au-akash
Copy link
Contributor

Description:
Adding RetrySettings and QueueSettings to Honeycomb Exporter
Adding these settings offered by the exporterhelper will allow users of the Honeycomb exporter to retain queued retry behavior

Testing:
RetrySettings and QueueSettings have been tested in exporterhelper, tests inside honeycomb exporter were changed to expect RetrySettings and QueueSettings

Documentation:
Added documentation on the new settings which can be configured on the exporter with descriptions on the defaults as well as their purpose

@au-akash au-akash requested a review from a team March 16, 2021 19:30
@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Mar 16, 2021

CLA Signed

The committers are authorized under a signed CLA.

@codecov
Copy link

codecov bot commented Mar 16, 2021

Codecov Report

Merging #2714 (e22bde2) into main (736647a) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2714      +/-   ##
==========================================
- Coverage   91.55%   91.54%   -0.01%     
==========================================
  Files         465      465              
  Lines       22848    22852       +4     
==========================================
+ Hits        20918    20920       +2     
- Misses       1437     1438       +1     
- Partials      493      494       +1     
Flag Coverage Δ
integration 68.71% <ø> (-0.25%) ⬇️
unit 90.51% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
exporter/honeycombexporter/factory.go 92.00% <100.00%> (+1.52%) ⬆️
receiver/k8sclusterreceiver/watcher.go 95.29% <0.00%> (-2.36%) ⬇️

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 736647a...e22bde2. Read the comment docs.

@bogdandrutu
Copy link
Member

@paulosman @lizthegrey @MikeGoldsmith please review

@paulosman
Copy link
Member

LGTM, thanks! @au-akash, you'll need to accept the CLA before the PR can be merged.

@au-akash
Copy link
Contributor Author

Thanks @paulosman! We're working on getting CLA approval

@au-akash au-akash force-pushed the as/add-queued-retry branch from 507e59a to 385eb54 Compare March 25, 2021 20:44
@au-akash au-akash force-pushed the as/add-queued-retry branch 4 times, most recently from 64ec1eb to 5c400eb Compare April 6, 2021 17:09
@au-akash
Copy link
Contributor Author

au-akash commented Apr 9, 2021

I signed it

@lizthegrey
Copy link
Member

Please rebase/commit --amend in order to force checks to run again

@au-akash au-akash force-pushed the as/add-queued-retry branch 2 times, most recently from 23cf856 to bf16ef5 Compare April 9, 2021 15:42
@au-akash
Copy link
Contributor Author

au-akash commented Apr 9, 2021

@lizthegrey I commit --amend, however the EasyCLA check is still failing.

This is how my linux foundation profile looks like:
image

@lizthegrey
Copy link
Member

@lizthegrey I commit --amend, however the EasyCLA check is still failing.

This is how my linux foundation profile looks like:
image

can you screenshot what happens when you visit https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/9522512/196414933/2714/#/?version=1

@au-akash au-akash force-pushed the as/add-queued-retry branch 2 times, most recently from 7c0a1ab to 7422dc5 Compare April 12, 2021 14:28
@au-akash au-akash force-pushed the as/add-queued-retry branch from 7422dc5 to e22bde2 Compare April 12, 2021 14:30
@au-akash
Copy link
Contributor Author

au-akash commented Apr 12, 2021

@lizthegrey @paulosman Sorry for the delay, CLA has now been approved.

Feel free to merge whenever you're ready 🙏

@bogdandrutu
Copy link
Member

Based on @paulosman approver merging this

@bogdandrutu bogdandrutu merged commit 9c52dda into open-telemetry:main Apr 12, 2021
pmatyjasek-sumo referenced this pull request in pmatyjasek-sumo/opentelemetry-collector-contrib Apr 28, 2021
alexperez52 referenced this pull request in open-o11y/opentelemetry-collector-contrib Aug 18, 2021
@james-bebbington is currently working on something else and cannot allocate time to the Collector. 
James, thanks for you work on the Collector, you are welcome back anytime in the future.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants