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

Body not present on instrumentation info.response object #76

Closed
roger-domenech-aguilera opened this issue Oct 21, 2021 · 9 comments · Fixed by #203
Closed

Body not present on instrumentation info.response object #76

roger-domenech-aguilera opened this issue Oct 21, 2021 · 9 comments · Fixed by #203

Comments

@roger-domenech-aguilera

Hi!

When calling the instrumentation function inside my custom metric, the response attribute of the info parameter does not have body.

def whatever() -> Callable[[Info], None]:
    metric = Gauge("whatever",
                   "This metric tracks whatever api usage.")

    def instrumentation(info: Info) -> None:
        request = info.request
        response = info.response
        body = response.body

    return instrumentation

AttributeError: 'StreamingResponse' object has no attribute 'body'

Is there a way to circumvent this issue or maybe I'm doing sth wrong!

Thank you all in advance!

@espositoandrea
Copy link

Hi! I've got the same issue here!
As a workaround I'm temporarily adding a custom header X-[foo] to hold the value that I would access from the body: don't know if that may help you for the time being 😄.

@abb4s
Copy link

abb4s commented Jan 8, 2022

same issue!!

def http_requested_languages_total():
    METRIC = Counter(
        "http_requested_languages_total", 
        "Number of times a certain language has been requested.", 
        labelnames=("spell_status",)
    )

    def instrumentation(info: Info) -> None:


        print(info.request)
        print(str(info.response.body_iterator))
        print(info.response.headers)
        # x = loads(info.response.body_iterator)["spell_status"]
        METRIC.labels(2).inc()

    return instrumentation
@app.get("/",response_class=JSONResponse)
def qpm(query: str,
        multi_suggest:Optional[bool] = True,
        normalization: Optional[bool] = True,
        synonym_handling: Optional[bool] = True,
        spell_correction: Optional[bool] = True,
        query_weighting: Optional[bool] = True,
        ignore_search_instead: Optional[bool] = True):

        
    return JSONResponse(content={
        "time_spell_correction":0,
        "time_query_weighting":0,
        "original_query": 0,
        "query": 0,
        "spell_correction_confidence": 0,
        "spell_status": 2,
    })

I want to create a new metric but the info.resonse object is a StreamResponse . how can i convert it to dict?

@tobikang
Copy link

Same issue.

@AbhishekBose
Copy link

Same issue. Body seems to be empty when I print it

@oysteins10
Copy link

oysteins10 commented Aug 18, 2022

Hey! I took a look in the prometheus middleware where it constructs the Info object. The middleware only grabs the headers and status code and constructs an empty bodied response object.
This snippet right here in the PrometheusInstrumentatorMiddleware.__call__ func in middleware.py (line 120):

...
response = Response(headers=Headers(raw=headers), status_code=status_code)  # type: ignore

info = metrics.Info(
             request=request,
             response=response,
             method=request.method,
             modified_handler=handler,
             modified_status=status,
             modified_duration=duration,
        )
...

I dont know why the authors did not include the response body as I know the the FastApi docs middleware docs uses Starlettes BaseHTTPMiddleware to grab the response object. Anyway, it's clear that the response body is not intended to be included. I also need to grab some metrics from the response body. One solution could be to create a custom api route that fills custom headers in the response object and then the instrumentator could pick these up.

@msemelman-vrff
Copy link

Hi I linked a PR trying to solve this issue. We are interested in getting this through. Can someone review please?

@trallnag trallnag linked a pull request Feb 22, 2023 that will close this issue
@bbeattie-phxlabs
Copy link
Contributor

Introduction of body data in this way kills performance for large FileResponse responses.

@trallnag
Copy link
Owner

There is a bug with this implementation. See issue #236.

It only works if response data is being streamed.

@trallnag
Copy link
Owner

FYI: There is a new release with a breaking change regarding this feature.

https://github.com/trallnag/prometheus-fastapi-instrumentator/releases/tag/v6.0.0

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

Successfully merging a pull request may close this issue.

9 participants