feat!: allow users to specify a trace policy impl#1027
feat!: allow users to specify a trace policy impl#1027kjin merged 11 commits intogoogleapis:masterfrom
Conversation
googlebot
added
the
cla: yes
kjin
force-pushed
the
new-trace-policy
branch
2 times, most recently
from
ac42747 to
a78929b
Compare
Codecov Report
@@ Coverage Diff @@
## master #1027 +/- ##
==========================================
- Coverage 94.81% 94.73% -0.08%
==========================================
Files 96 96
Lines 6186 6226 +40
Branches 481 486 +5
==========================================
+ Hits 5865 5898 +33
- Misses 166 172 +6
- Partials 155 156 +1
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #1027 +/- ##
==========================================
+ Coverage 94.81% 94.89% +0.08%
==========================================
Files 96 96
Lines 6186 6226 +40
Branches 481 484 +3
==========================================
+ Hits 5865 5908 +43
+ Misses 166 161 -5
- Partials 155 157 +2
Continue to review full report at Codecov.
|
BREAKING CHANGE: contextHeaderBehavior and ignoreContextHeader now act independently of one another. The former controls how a sampling decision is made based on incoming context header, and the latter controls whether trace context is propagated to the current request.
| * For advanced usage only. | ||
| * If specified, overrides the built-in trace policy object. Note that | ||
| * ignoreUrls, ignoreMethods, samplingRate, and contextHeaderBehavior will be | ||
| * ignored if this field is specified. |
There was a problem hiding this comment.
Add a throw if any of ignored fields are provided along with tracePolicy.
There was a problem hiding this comment.
I've made it a warning... AFAICT we try not to throw, right?
There was a problem hiding this comment.
This clearly indicates user error. This is a good example for when one should throw.
There was a problem hiding this comment.
Ok, makes sense. I've changed it to throw instead.
This reverts commit 3893059.
| } | ||
|
|
||
| export interface TracePolicy { | ||
| shouldTrace: (requestDetails: RequestDetails) => boolean; |
There was a problem hiding this comment.
It seems like boilerplate to pass in an object wrapping a funciton. Could you just accept a function?
There was a problem hiding this comment.
I'd rather we accept the object. With propagation we would have another overriding class Propagation where it makes more sense to have an object, because there are three fields. This would also allow users to easily implement a class and pass in an instance of a class.
3 participants
Fixes #994
This PR allows users to override the built-in
TracePolicyobject with their own implementation. Such an implementation has the following interface:It also makes a breaking change to the config option:
contextHeaderBehaviornow only affects sampling.propagateTraceContextFromHeaderhas been added to allow users to ignore trace context when creating new trace and span IDs.ignoreContextHeaderhas been removed. It was already deprecated. (propagateTraceContextFromHeader) is meant to represent the semantic opposite of it.Internally, a
TracePolicyobject is now instantiated outside ofStackdriverTracerand passed in (it used to be instantiated by theStackdriverTracerinstance).