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

Define future approach to metadata and metrics with new Executor #1362

Closed
RafalSkolasinski opened this issue Jan 23, 2020 · 5 comments
Closed
Assignees

Comments

@RafalSkolasinski
Copy link
Contributor

RafalSkolasinski commented Jan 23, 2020

What this issue is about

Very soon Executor, in Go replacement for currently used Java engine, will be merged into master.

In this context we need to define our future approach to metadata and metrics. How it is gonna be ideally handled in the future and how are we gonna avoid breaking backward compatibility during period of time when we support both Executor and Engine.

In order to help discussion I did try to put together a summary of the current state of matter.
As the topic seems to cause some confusion, especially on the semantic level, the description of this issue may be a bit lengthy, apologies for that.

Definitions

Metadata (model):

  • Model specific information: model name, author, training dataset, input's shape and type, etc...
  • May or may not be dynamic (depending on whether it is exposed)
  • Does not depend on incoming requests

Metadata (request):

  • This is the metadata that is sent with each request / response
  • This is the metadata of the request itself (e.g. sender or unique identifier)
  • May depend on external resources (e.g. external api version on time of request processing)
  • In the future may be included in the header of the HTTP request
  • It is part of the SeldonMessage

Metrics:

  • Should represent current state of model / deployment
  • Standard devops metrics (like number of requests per second, cpu usage) we provide by default and expose via endpoint for Prometheus
  • User defined custom metrics is returned with requests (Seldon Engine)
  • In the executor each container will expose its own metrics endpoint (picked up by Prometheus)

Now: only Java Engine

User-provided models can define two methods: metrics and tags.
Both of these are included in response.meta when /predict end point is called.
All of that logic is defined in the Python wrapper, not in Java engine.

Metrics method:

  • Return type: list of dictionaries
  • Directly used in user_model.py::client_custom_metrics method
  • Returned in response.meta.metrics of SeldonMessage
  • Commonly used to for custom metrics defined by user

Tags method:

  • Return type: dictionary
  • Directly used in user_model.py::client_custom_tags method
  • Returned in response.meta.tags of SeldonMessage
  • Commonly used to return request metadata
  • Problem: predict method must modify current state of model in order to provide data returned by this method

Future: compatibility with both Executor and Engine

For some time we will be supporting both Executor and Engine.
Python wrapper must be therefore compatible with both.
Wrapper will know if it is working with Executor or Engine through environmental variables.

Go Executor:

Metadata:

  • User-provided models can define a metadata method.
  • Output from that method will be exposed via /metadata endpoint.
  • It contains pure model metadata in the context of the above definitions.

Metrics:

  • Executor’s /metadata endpoint will only contain general metrics
  • Custom metrics will be exposed on /metadata of each container (and consumed by Prometheus)

Java Engine:

Main compatibility with the current functionality until we deprecate Java Engine.

Problems / objectives

  • What to do with current metrics and tags method? Shall these get deprecated?”
  • How to provide backward compatibility with models defined to work with pre-executor seldon-core? These may use metrics and tags in a non-trivial manner.
  • Shall per-request metadata be appended to response?

Main questions:

  • How do we want to handle these things (especially custom metrics) in the future?
  • How do we keep compatibility with earlier defined models?

P.S. I did my best to get these points right. However, if something is wrong or confusing, please, leave a comment or let me know and we'll try to clarify it.

@RafalSkolasinski RafalSkolasinski added the triage Needs to be triaged and prioritised accordingly label Jan 23, 2020
@RafalSkolasinski
Copy link
Contributor Author

Ideally, this issue will lead us to a clear new picture on how we want to handle these things or set of well defined issues that precisely describe desired functionality.

@RafalSkolasinski
Copy link
Contributor Author

RafalSkolasinski commented Jan 27, 2020

I believe equivalent of ours request metadata in kfserving dataplane proposal would be $parameters present in $inference_request, $request_input, $request_output and $inference_response.

Actually, I think I was not exactly correct. Seems that $parameters are more about controlling inference process.

Not sure then if request metadata is in any way present in this dataplane proposal.

@RafalSkolasinski
Copy link
Contributor Author

The request metadata is in principle metadata about the request itself.

It can be immutable data that comes as a part of the request. In example:

  • some id of user who send the request (for audit purposes)
  • some unique request identifier (could be useful in "Batch" processing)

This data should be exposed to predict method but guaranteed to be immutable.

The other example of request metadata provided by @adriangonz is recording version of external libraries accessed during the time of processing the request. This type of data is:

  • unknown to sender on time of the request
  • may change in between requests

We should provide an easy and threadsafe way to include that kind of metadata in the request response.

@RafalSkolasinski
Copy link
Contributor Author

Reference: kfserving dataplane proposal

@ukclivecox ukclivecox removed this from the 1.2 milestone Apr 23, 2020
@ukclivecox
Copy link
Contributor

already in progress in other issues

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

2 participants