Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changing "sidecar.jaegertracing.io/inject" annotation to be more generic #351

Closed
objectiser opened this issue Apr 2, 2019 · 4 comments
Closed
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest

Comments

@objectiser
Copy link
Contributor

Currently the agent sidecar injection mechanism relies on the deployment including an annotation sidecar.jaegertracing.io/inject with value either true or the name of the jaeger instance it is associated with.

As well as injecting the agent, it also adds a couple of environment variables (service name and propagation) to automatically configure the tracer used by the service.

When the agent strategy is set to DaemonSet, the user becomes responsible for configuring the environment variables.

Wondering if we could make the annotation more generic - so not associated with the concept of "inject" and "sidecar", and instead be related to just indicating the deployment should be instrumented (or observed).

Then the operator can use knowledge of whether a sidecar or daemonset agent is being used, and configure the deployment accordingly - along with the relevant environment variables for the selected strategy.

@jpkrohling
Copy link
Contributor

The agent strategy isn't an "and/or" decision: there can be a daemon set for for most applications and sidecars dedicated to certain applications. One further argument in favor of the current naming is that its outcome is similar to the similarly named annotation from Istio.

But we could certainly consider a second, non-sidecar/inject related annotation, that will enhance the pod with env vars such as JAEGER_SERVICE_NAME.

@jpkrohling jpkrohling added enhancement New feature or request good first issue Good for newcomers labels Apr 2, 2019
@objectiser
Copy link
Contributor Author

Is the sidecar injection "always on"? If so, then I think we need to update the documentation to make this clear, as the implication currently is that strategy default is inject but can be changed to daemonset - i.e. either or.

If that is the case, then possibly a separate annotation would be good - whether the sidecar.jaeger.io/inject implies this other annotation or not is a separate question.

@jpkrohling
Copy link
Contributor

Need to check the code, but IIRC, the only difference when the strategy is set to DaemonSet is that we deploy the daemon set.

@jpkrohling jpkrohling added needs-triage New issues, in need of classification and removed needs-triage New issues, in need of classification labels Dec 16, 2019
@pavolloffay
Copy link
Member

Closing, the agent component of the Jaeger deployment might get replaced with OpenTelemetry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers hacktoberfest
Projects
None yet
Development

No branches or pull requests

3 participants