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

Allow access to puid within the predict API #795

Closed
pisymbol opened this issue Aug 13, 2019 · 6 comments
Closed

Allow access to puid within the predict API #795

pisymbol opened this issue Aug 13, 2019 · 6 comments
Assignees
Milestone

Comments

@pisymbol
Copy link

Every prediction should come with a transaction ID. This ID could be used for statistics as well as a way to uniquely identify a thread of inference execution (worker process under gunicorn) etc.

Right now, that transaction ID is essentially the puid under the raw Seldon Message which is only accessible via the predict_raw entry point NOT the predict one. It does get returned in the 'meta' but that is not helpful at run-time.

Seldon Core should expose the puid (or some other transaction ID) that model developers can read at the time of each predict call.

Small but related corollary: I actually think an entire Seldon Context should be created and be available at the time of a "predict" call and it could house information such as this transaction ID as well as other useful information such as request IP, custom parameters added via deployment annotations etc. I envision a world for instance where I deploy a model under Seldon and depending on some deployment annotation (say a version string or a debug mode) I can change the model behavior without ever making a single code change. Currently, the only real way to dynamically do this is via sending a model version string as a feature or a canary type deployment (kinda lame if you ask me).

@phsiao
Copy link

phsiao commented Aug 13, 2019

It might be helpful to pass opentracing active span via the context also.

For example, in the python model wrapper, I can't find a clean way to pass the span that flask created to the model's functions, so we end up with two disjoint set of traces.

@ukclivecox ukclivecox added this to the 1.0.x milestone Aug 24, 2019
@adriangonz
Copy link
Contributor

@pisymbol there is an optional meta parameter that you can receive on your predict() method. You can find some documentation about it here: https://docs.seldon.io/projects/seldon-core/en/latest/python/python_component.html#model.

This meta object should be the same as the one within the request object passed to predict_raw(). Have you checked if the puid is present there?

@adriangonz
Copy link
Contributor

@phsiao the opentracing instance inside the Python model wrapper should already pick up the opened span from the service orchestrator.

For example, you can see the following trace where the context was created on the orchestrator (first two traces in yellow) and a 3rd trace was submitted from the Python wrapper (the last one in blue).

Working Trace

@adriangonz
Copy link
Contributor

@pisymbol have you had a chance to check if the puid is present when you also pass the meta parameter to your predict(...) method?

@ukclivecox
Copy link
Contributor

Please reopen if still an issue

@pisymbol
Copy link
Author

Reopen please! As of 0.5.1, puid is still not available in 'meta' within predict. Verified today.

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

4 participants