[Inference API] Auto-propagate product origin for every subclass of ElasticInferenceServiceRequest #123141
Conversation
timgrein
added
>non-issue
:ml
|
Pinging @elastic/ml-core (Team:ML) |
| @Override | ||
| public final HttpRequest createHttpRequest() { | ||
| HttpRequestBase request = createHttpRequestBase(); | ||
| // TODO: consider moving tracing here, too |
There was a problem hiding this comment.
Wanted to keep it out of scope for this PR, but I think it also makes sense to move the tracing logic here, so we don't risk forgetting it for new subclasses of ElasticInferenceServiceRequest.
| } | ||
|
|
||
| @Override | ||
| public final HttpRequest createHttpRequest() { |
There was a problem hiding this comment.
Made this final so it's clear that a new subclass of ElasticInferenceServiceRequest should override createHttpRequestBase and not createHttpRequest.
| public final HttpRequest createHttpRequest() { | ||
| HttpRequestBase request = createHttpRequestBase(); | ||
| // TODO: consider moving tracing here, too | ||
| request.setHeader(Task.X_ELASTIC_PRODUCT_ORIGIN_HTTP_HEADER, productOrigin); |
There was a problem hiding this comment.
Is productOrigin null if it isn't present in the inbound header? I can't tell if defaultHeaders would include a default productOrigin, but if it doesn't we may want to either omit the header or send something like "Unknown"
There was a problem hiding this comment.
I think we don't really need to care as EIS handles the case, if this header is absent and/or empty and sets unknown if so. I would like to simply treat this as a forwarding logic without looking into the value.
💚 All backports created successfully
Questions ?Please refer to the Backport tool documentation |
…lasticInferenceServiceRequest (elastic#123141) (cherry picked from commit 08aa668)
…lasticInferenceServiceRequest (elastic#123141)
3 participants
Many systems at Elastic forward their "product origin" (usually the name of the system itself) throughout a specific HTTP header, namely
X-Elastic-Product-Origin. It makes sense to also have this metadata on EIS to add it as metadata to the usage API calls and/or to APM attributes for better insights/traceability.I've implemented it in a way that each new subclass of
ElasticInferenceServiceRequest(which we'll have as soon as we support new task types) doesn't need to explicitly add the header to reduce the risk of us forgetting it.