Skip to content
This repository has been archived by the owner on Jul 1, 2022. It is now read-only.

Java: Cleanup operation samplers if absent from strategy response #655

Merged
merged 1 commit into from
Oct 2, 2019

Conversation

guo0693
Copy link
Contributor

@guo0693 guo0693 commented Oct 2, 2019

Signed-off-by: jung jung@uber.com

Which problem is this PR solving?

Short description of the changes

  • If the server did not return a strategy for operation X, the operation sampler will be removed and the default sampler will be used for operation X

@guo0693 guo0693 changed the title Cleanup operation samplers if absent from strategy response Java: Cleanup operation samplers if absent from strategy response Oct 2, 2019
String absentOp = "ShouldBeRemoved";
operationToSamplers.put(absentOp, mock(GuaranteedThroughputSampler.class));
PerOperationSamplingParameters perOperationSamplingParameters1 =
new PerOperationSamplingParameters(OPERATION, new ProbabilisticSamplingStrategy(SAMPLING_RATE));
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't operationToSamplers be passed in here to control the starting state of the test?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

undertest is inited with the operationToSamplers during setup, adding a sampler to the map should do the job regarding the starting state.

@guo0693 guo0693 closed this Oct 2, 2019
@guo0693 guo0693 reopened this Oct 2, 2019
@codecov
Copy link

codecov bot commented Oct 2, 2019

Codecov Report

Merging #655 into master will decrease coverage by 0.08%.
The diff coverage is 100%.

Impacted file tree graph

@@             Coverage Diff             @@
##             master    #655      +/-   ##
===========================================
- Coverage     89.48%   89.4%   -0.09%     
  Complexity      563     563              
===========================================
  Files            69      69              
  Lines          2073    2076       +3     
  Branches        263     263              
===========================================
+ Hits           1855    1856       +1     
- Misses          136     138       +2     
  Partials         82      82
Impacted Files Coverage Δ Complexity Δ
...tracing/internal/samplers/PerOperationSampler.java 93.87% <100%> (+0.39%) 18 <1> (ø) ⬇️
...egertracing/internal/reporters/RemoteReporter.java 82.55% <0%> (-2.33%) 7% <0%> (ø)

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 21ea7af...8a5720e. Read the comment docs.

@vprithvi vprithvi merged commit 0961849 into jaegertracing:master Oct 2, 2019
@guo0693 guo0693 deleted the #1728 branch October 3, 2019 15:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adaptive sampling strategies not updated in certain situations
2 participants