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

Bump Cloud Trace library to v1.8.0 #550

Merged
merged 7 commits into from
Jan 27, 2023

Conversation

damemi
Copy link
Contributor

@damemi damemi commented Dec 21, 2022

Fixes #208

This bumps the cloud trace client to v1.8.0, which offers default batch retry on failed writes.

Currently the trace client retries failed calls for CreateSpans but not BatchWriteSpans.

@dashpole
Copy link
Contributor

Should we open an upstream issue asking for the default to be changed? Or ask why the default is set the way it is?

If we can avoid adding a new config surface, that seems best. It also would seem odd for us to have a different default from the upstream library.

@damemi
Copy link
Contributor Author

damemi commented Dec 21, 2022

Upstream issue raised at googleapis/google-cloud-go#7184

@damemi
Copy link
Contributor Author

damemi commented Jan 10, 2023

update: we implemented this as a default setting in the upstream trace client instead. but, I think we can still use the unit tests I added here to make sure attempts are retried. once the upstream generated clients land, I'll use this PR to bump the dependency and add these tests. Holding until then

@damemi damemi changed the title Add WithBatchRetry option to trace exporter Bump Cloud Trace library to v1.8.0 Jan 27, 2023
@codecov
Copy link

codecov bot commented Jan 27, 2023

Codecov Report

Merging #550 (8f4a511) into main (9c0f81b) will increase coverage by 0.00%.
The diff coverage is 37.50%.

@@           Coverage Diff           @@
##             main     #550   +/-   ##
=======================================
  Coverage   64.68%   64.68%           
=======================================
  Files          38       38           
  Lines        4281     4282    +1     
=======================================
+ Hits         2769     2770    +1     
  Misses       1399     1399           
  Partials      113      113           
Impacted Files Coverage Δ
...er/collector/integrationtest/testcases/testcase.go 82.87% <ø> (ø)
exporter/collector/logs.go 41.23% <ø> (ø)
exporter/collector/metrics.go 67.68% <ø> (ø)
exporter/metric/metric.go 72.38% <ø> (ø)
exporter/trace/trace.go 90.90% <ø> (ø)
exporter/trace/trace_proto.go 64.73% <ø> (ø)
...er/collector/integrationtest/protos/fixtures.pb.go 42.48% <28.57%> (ø)
exporter/collector/traces.go 70.00% <100.00%> (+0.61%) ⬆️
...lector/internal/datapointstorage/datapointcache.go 83.33% <0.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@damemi
Copy link
Contributor Author

damemi commented Jan 27, 2023

Working on deprecated import path migration (https://github.com/googleapis/google-cloud-go/blob/main/migration.md)...

@damemi damemi merged commit 720e1b5 into GoogleCloudPlatform:main Jan 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.

opentelemetry-operations-go/exporter/trace rpc error: code = Unavailable
2 participants