[GitHub] [skywalking] wu-sheng commented on a change in pull request #5407: Save error trace segment even that segment will abandoned by sampling mechanism

[GitBox <gi...@apache.org>]

wu-sheng commented on a change in pull request #5407:
URL: https://github.com/apache/skywalking/pull/5407#discussion_r478869479



##########
File path: docs/en/setup/backend/trace-sampling.md
##########
@@ -30,4 +30,8 @@ When you set the rate different, let's say
 And we assume the agents reported all trace segments to backend,
 Then the 35% traces in the global will be collected and saved in storage consistent/complete, with all spans.
 20% trace segments, which reported to Backend-Instance**B**, will saved in storage, maybe miss some trace segments,
-because they are reported to Backend-Instance**A** and ignored.
\ No newline at end of file
+because they are reported to Backend-Instance**A** and ignored.
+
+# Note
+When you open sampling, the actual sampleRate will above sampleRate. Because we want some error segment will be saved, even that segment will abandoned by server side trace sampling mechanism. may be miss some other trace segments, but we can analyze this error segment to solve problem.

Review comment:
       Try to provide a configuration to disable this feature, this could be default open as the harm is limited.

##########
File path: docs/en/setup/backend/trace-sampling.md
##########
@@ -30,4 +30,8 @@ When you set the rate different, let's say
 And we assume the agents reported all trace segments to backend,
 Then the 35% traces in the global will be collected and saved in storage consistent/complete, with all spans.
 20% trace segments, which reported to Backend-Instance**B**, will saved in storage, maybe miss some trace segments,
-because they are reported to Backend-Instance**A** and ignored.
\ No newline at end of file
+because they are reported to Backend-Instance**A** and ignored.
+
+# Note
+When you open sampling, the actual sampleRate will above sampleRate. Because we want some error segment will be saved, even that segment will abandoned by server side trace sampling mechanism. may be miss some other trace segments, but we can analyze this error segment to solve problem.

Review comment:
       ```suggestion
   When you open sampling, the actual sample rate could be over sampleRate. Because currently, all error segments will be saved, meanwhile, the upstream and downstream may not be sampled. This feature is going to make sure you could have the error stacks and segments, but don't guarantee you would have the whole trace.
   
   Also, the side effect would be, if most of the accesses are fail, the sampling rate would be closing to 100%, which could crash the backend or storage clusters.
   ```




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org