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

Catchup with otel version since stackdriver released 0.17.0 #2613

Merged
merged 2 commits into from
Mar 9, 2021

Conversation

bogdandrutu
Copy link
Member

@bogdandrutu bogdandrutu commented Mar 8, 2021

Bundle support was removed from Stackdriver exporter, enable queue/retry as we do in all the other exportes.

Signed-off-by: Bogdan Drutu [email protected]

@bogdandrutu bogdandrutu requested a review from a team as a code owner March 8, 2021 19:34
@bogdandrutu bogdandrutu force-pushed the catchupotel branch 3 times, most recently from f1a2c49 to d2385c4 Compare March 8, 2021 20:17
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.

Thanks for doing this!

@jsuereth
Copy link
Contributor

jsuereth commented Mar 8, 2021

@bogdandrutu There are two components to this PR I'd like a chance to deal with:

  1. The README.md for the exporter should be updated to reflect the configuration changes.
  2. I'd like to add support for still taking in bundler options and translating them (but warning the user they're deprecated). Should I append to this PR?

@codecov
Copy link

codecov bot commented Mar 8, 2021

Codecov Report

Merging #2613 (102bc06) into main (16d8ca9) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #2613   +/-   ##
=======================================
  Coverage   91.32%   91.32%           
=======================================
  Files         431      431           
  Lines       21480    21451   -29     
=======================================
- Hits        19616    19591   -25     
+ Misses       1395     1392    -3     
+ Partials      469      468    -1     
Flag Coverage Δ
integration 69.24% <ø> (ø)
unit 90.23% <100.00%> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
exporter/stackdriverexporter/factory.go 81.81% <100.00%> (+1.81%) ⬆️
exporter/stackdriverexporter/spandata.go 83.83% <100.00%> (ø)
exporter/stackdriverexporter/stackdriver.go 77.22% <100.00%> (-0.81%) ⬇️
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 16d8ca9...102bc06. Read the comment docs.

@bogdandrutu
Copy link
Member Author

@jsuereth feel free to update this PR :)

@jsuereth
Copy link
Contributor

jsuereth commented Mar 8, 2021

Cool, I'll be updating this PR to avoid a release with a breaking change with no deprecation period. Hopefully will finish before your morning tomorrow.

Thanks again for adding the exporter helper support!!!

@jsuereth
Copy link
Contributor

jsuereth commented Mar 9, 2021

managed to eek this out tonight: bogdandrutu#933

@bogdandrutu
Copy link
Member Author

@jsuereth updated and added the feedback :)

@jsuereth
Copy link
Contributor

jsuereth commented Mar 9, 2021

Thanks again for making these changes!

@bogdandrutu bogdandrutu merged commit d58b47e into open-telemetry:main Mar 9, 2021
This was referenced Mar 15, 2021
kisieland pushed a commit to kisieland/opentelemetry-collector-contrib that referenced this pull request Mar 16, 2021
* [prometheusremotewrite] fix: counter name check

fixes  #2608

Ensure that the metric name is greater than the length of the
counter suffix ("_total") before checking if it contains the counter
suffix to prevent crashes for short (smaller or eq to the length of the
suffix) metric names.

The edge case where the metric name is "_total" is not handled (considered
not to have the suffix), but should not occur IRL.

Also fix the spelling of "delimiter".

Signed-off-by: naseemkullah <[email protected]>

* use strings.HasSuffix()

Signed-off-by: naseemkullah <[email protected]>

* add already suffixed counter test

Signed-off-by: naseemkullah <[email protected]>
pmatyjasek-sumo referenced this pull request in pmatyjasek-sumo/opentelemetry-collector-contrib Apr 28, 2021
* Catchup with otel version since stackdriver released 0.17.0

Signed-off-by: Bogdan Drutu <[email protected]>

* Update README

Signed-off-by: Bogdan Drutu <[email protected]>
ljmsc referenced this pull request in ljmsc/opentelemetry-collector-contrib Feb 21, 2022
* Refactor Makefile

* Update dependabot targets

* Sync github actions config with Makefile ci target

* Update test targets

* Use sed instead of parameter indexing

* Remove dependabot-generate

Address in #2613 instead.
ljmsc referenced this pull request in ljmsc/opentelemetry-collector-contrib Feb 21, 2022
* Refactor common repo code for crosslink

* Add dbotconf utility

* Add dependabot-generate target to Makefile

* Generate dependabot.yml

* Update Makefile targets related to dependabot-generate
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.

5 participants