Skip to content

Fix flaky gRPC server instrumentation#1122

Merged
SylvainJuge merged 7 commits intoelastic:masterfrom
SylvainJuge:fix-flaky-grpc-tests
Apr 6, 2020
Merged

Fix flaky gRPC server instrumentation#1122
SylvainJuge merged 7 commits intoelastic:masterfrom
SylvainJuge:fix-flaky-grpc-tests

Conversation

@SylvainJuge
Copy link
Member

@SylvainJuge SylvainJuge commented Apr 3, 2020

What does this PR do?

A test was qualified as flaky, whereas in practice the implementation was the culprit.

gRPC server instrumentation was relying on the fact that transaction processing stayed on the same thread. However, even if it is the case most of the time, it is not an absolute truth and produces random failures.

Thus, I've reworked gRPC server instrumentation to store in-flight transaction within helper class in-memory storage (a hash-map), in a similar way as it was done for client instrumentation.

Checklist

  • My code follows the style guidelines of this project
  • I have rebased my changes on top of the latest master branch
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • I have updated CHANGELOG.asciidoc
  • I have updated supported-technologies.asciidoc
  • Added an API method or config option? Document in which version this will be introduced
  • Added an instrumentation plugin? How did you make sure that old, non-supported versions are not instrumented by accident?

Author's Checklist

  • Run the tests locally continuously for 10min without any failure

Related issues

@SylvainJuge SylvainJuge changed the title Fix flaky grpc server instrumentation Apr 3, 2020
@SylvainJuge SylvainJuge requested a review from eyalkoren April 3, 2020 16:24
@SylvainJuge SylvainJuge mentioned this pull request Apr 3, 2020
@codecov-io
Copy link

Codecov Report

Merging #1122 into master will decrease coverage by 0.27%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1122      +/-   ##
============================================
- Coverage     60.04%   59.76%   -0.28%     
  Complexity       87       87              
============================================
  Files           327      327              
  Lines         14806    14867      +61     
  Branches       2071     2083      +12     
============================================
- Hits           8890     8886       -4     
- Misses         5318     5381      +63     
- Partials        598      600       +2     
Impacted Files Coverage Δ Complexity Δ
...m/agent/grpc/ServerCallHandlerInstrumentation.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...stic/apm/agent/grpc/ServerCallInstrumentation.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
.../agent/grpc/ServerCallListenerInstrumentation.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
.../elastic/apm/agent/grpc/helper/GrpcHelperImpl.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...va/co/elastic/apm/agent/impl/ElasticApmTracer.java 71.14% <0.00%> (-1.01%) 0.00% <0.00%> (ø%)
...o/elastic/apm/agent/profiler/SamplingProfiler.java 77.93% <0.00%> (-0.28%) 0.00% <0.00%> (ø%)

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 ef369a1...b64207e. Read the comment docs.

@SylvainJuge SylvainJuge merged commit a0e0abd into elastic:master Apr 6, 2020
@SylvainJuge SylvainJuge deleted the fix-flaky-grpc-tests branch April 6, 2020 13:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

4 participants