add env var JAEGER_DISABLED #1285
Conversation
Signed-off-by: Simon Schneider <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #1285 +/- ##
==========================================
+ Coverage 87.32% 87.36% +0.03%
==========================================
Files 89 89
Lines 4947 4970 +23
==========================================
+ Hits 4320 4342 +22
- Misses 464 465 +1
Partials 163 163
Continue to review full report at Codecov.
|
| // JaegerDisabled adds the JAEGER_DISABLED environment flag to the | ||
| // query component to prevent it from adding its own traces. | ||
| // The default, if ommited, is false | ||
| JaegerDisabled *bool `json:"jaegerDisabled,omitempty"` |
There was a problem hiding this comment.
Would it be better calling it tracingEnabled - with a default of true?
| // JaegerDisabled adds the JAEGER_DISABLED environment flag to the | ||
| // query component to prevent it from adding its own traces. | ||
| // The default, if ommited, is false | ||
| JaegerDisabled *bool `json:"jaegerDisabled,omitempty"` |
Signed-off-by: Simon Schneider <[email protected]>
Where should this be implemented? Directly in inject.Sidecar()? |
|
@sschne Just don't include the |
Signed-off-by: Simon Schneider <[email protected]>
|
@objectiser thanks for the hint, but that was not enough. The Strategy would inject the sidecar regardless of the annotation, so there is will be a race between the Strategy and namespace controller. Now i also prevented the strategy from injecting the sidecar. |
| queryDep := inject.Sidecar(jaeger, inject.OAuthProxy(jaeger, query.Get())) | ||
| queryDep := inject.OAuthProxy(jaeger, query.Get()) | ||
| if jaeger.Spec.Query.TracingEnabled == nil || *jaeger.Spec.Query.TracingEnabled == true { | ||
| queryDep = inject.Sidecar(jaeger, queryDep) |
There was a problem hiding this comment.
@jpkrohling should it be necessary to explicitly add the sidecar if the annotation has been defined? If so, what is the purpose of adding the annotation?
There was a problem hiding this comment.
We can then avoid the query deployment from being created without the sidecar, only to get the sidecar injected on the next reconciliation for the Deployment resource.
|
Thanks @sschne |
Adds option to set the environment variable JAEGER_DISABLED to query or all-in-one deployment.
Closes: #1148
Signed-off-by: Simon Schneider [email protected]