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

executor tracing enhancement #1444

Closed
andrewshaoyu opened this issue Feb 14, 2020 · 5 comments
Closed

executor tracing enhancement #1444

andrewshaoyu opened this issue Feb 14, 2020 · 5 comments

Comments

@andrewshaoyu
Copy link
Contributor

Using Seldon with istio , envoy can propagate the trace to executor. If start a trace from client , we can see the whole trace from client to envoy then executor , then predict model.
but now some work need to be done.

  1. now envoy propagate trace by three ways with different headers. like zipkinb3, watchDog...so executor need to support the different propagation. I can make a pr.
  2. how to inject the jaeger envs to executor. I think we can add the env to seldon-config configmap, operator get the envs then patch to executor env.

how about the second point?

@ukclivecox
Copy link
Contributor

Using Seldon with istio , envoy can propagate the trace to executor. If start a trace from client , we can see the whole trace from client to envoy then executor , then predict model.
but now some work need to be done.

1. now envoy propagate trace by three ways with different headers. like zipkinb3, watchDog...so executor need to support the different propagation. I can make a pr.

A PR would be great!

2. how to inject the jaeger envs to executor. I think we can add the env to seldon-config configmap, operator get the envs then patch to executor env.

how about the second point?

The current way for point 2 is shown in https://docs.seldon.io/projects/seldon-core/en/latest/graph/distributed-tracing.html

Where you add

 "svcOrchSpec" : {
            "env": [
            {
                "name": "TRACING",
                "value": "1"
            },
            {
                "name": "JAEGER_AGENT_HOST",
                "value": "jaeger-agent"
            },
            {
                "name": "JAEGER_AGENT_PORT",
                "value": "5775"
            },
            {
                "name": "JAEGER_SAMPLER_TYPE",
                "value": "const"
            },
            {
                "name": "JAEGER_SAMPLER_PARAM",
                "value": "1"
            }
            ]
        }

@ryandawsonuk
Copy link
Contributor

ryandawsonuk commented Feb 14, 2020

@andrewshaoyu I think it would be cool if we could make it a configuration like

PROPAGATE_HEADERS="header1,header2,header3"

Then it could be used for tracing or other use-cases like auth headers. I found some examples of other projects taking this broad approach:

eclipse/microprofile-rest-client#73
https://aws.amazon.com/premiumsupport/knowledge-center/configure-cloudfront-to-forward-headers/
grpc-ecosystem/grpc-gateway#253

It could be called 'whitelist headers' instead of 'propagate headers' and could be an annotation or env var.

Or if instead you want to do something specific to tracing I think that's ok too.

@ukclivecox
Copy link
Contributor

I think the question for tracing is if a header provides the Span ID what is the best practice for further propogating the headers. Presently all headers are propogated to models in the executor but maybe we want to stop certain headers like tracing headers from further propogation as new headers will be added for this.

@andrewshaoyu
Copy link
Contributor Author

i have commited the pr. #1445

@ukclivecox
Copy link
Contributor

Fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants