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

traces_sampler receives always "generic ASGI request" as transaction name. #2262

Closed
antonpirker opened this issue Jul 19, 2023 · 3 comments · Fixed by #2341
Closed

traces_sampler receives always "generic ASGI request" as transaction name. #2262

antonpirker opened this issue Jul 19, 2023 · 3 comments · Fixed by #2341

Comments

@antonpirker
Copy link
Member

How do you use Sentry?

Sentry Saas (sentry.io)

Version

1.11.1

Steps to Reproduce

I have manually instrumented traces_sampler counted on transaction name in sampling context. At some point with sentry-sdk version upgrade, it stopped work - there is always "generic ASGI request" in transaction name. But at the performance page, transaction has a proper name.

How we initiate sentry FastAPI integration:

sentry_sdk.init(
    dsn=SENTRY_DSN,
    environment="dev",
    traces_sampler=sentry_traces_sampler,
    integrations=[
        starlette.StarletteIntegration(transaction_style="endpoint"),
        fastapi.FastApiIntegration(transaction_style="endpoint"),`
    ]
)

The traces_sampler function:

def sentry_traces_sampler(sampling_context):
    transaction_sampling_mapper = {
        "app.api.api_v1.routers.users.some_name": 0.1,
        "app.api.api_v1.routers.messages.some_name_2": 0.5,
    }
    return transaction_sampling_mapper.get(
        sampling_context["transaction_context"]["name"], 1.0
    )

Expected Result

Actual Result

Possible solution: Give the plain URL of the request instead of "generic ASGI request" as the transaction name to traces_sampler.

@antonpirker
Copy link
Member Author

Related to #1599

@antonpirker
Copy link
Member Author

The problem here is, that we give the event to the traces_sampler as soon as possible. At this point in time the Starlette or FastAPI integrations have not yet set the transaction name to the route (because they have not yet run)

To give the traces_sampler something to work with, we will set the transaction name to the URL (which later will be overwritten by the Starlette/FastAPI integrations with the route. (because transaction names need to have low cardinality, we can not leave the URL as the name, because it will mess up grouping)

@antonpirker
Copy link
Member Author

Oh, I just remembered that @polina-popova commented in this issue that it used to work at some point: #1599 (comment)

antonpirker added a commit that referenced this issue Sep 6, 2023
Set the url as a transaction name instead of 'generic ASGI request' in the beginning, so traces_sampler has something to work with that is more meaningful than 'generic ASGI request'.

Closes #2262
Closes #2263
New Behaviour:

Note: transaction names can be two styles, "url" or "endpoint". (set by the transaction_style parameter of the Integrations)

Note 2: See also @pytest.mark.parametrize decorator in the new tests as reference.

    vanilla ASGI: set URL instead of always "generic ASGI request"
    Starlette:
        normal request: transaction name is function name or route (depending on transaction_style setting)
        traces_sampler: always receives the raw URL as the transaction name (no matter the transaction_style setting. because we do not know more at the time the traces_sampler is called.)
        requests that end in a middleware (like 404, CORS): the functions name or the raw URL (depending on transaction_style setting)
    FastAPI
        normal request: transaction name is function name or route (depending on transaction_style setting)
        traces_sampler: always receives the raw URL as the transaction name (no matter the transaction_style setting. because we do not know more at the time the traces_sampler is called.)
        requests that end in a middleware (like 404, CORS): the functions name or the raw URL (depending on transaction_style setting)
    There used to be "generic ASGI request" transactions being created at the server startup (when a "lifespan" ASGI message was received.) Those transactions are not created anymore. (we can think of creating propper "Server was started/stopped" transactions in the future)
sentrivana pushed a commit that referenced this issue Sep 18, 2023
Set the url as a transaction name instead of 'generic ASGI request' in the beginning, so traces_sampler has something to work with that is more meaningful than 'generic ASGI request'.

Closes #2262
Closes #2263
New Behaviour:

Note: transaction names can be two styles, "url" or "endpoint". (set by the transaction_style parameter of the Integrations)

Note 2: See also @pytest.mark.parametrize decorator in the new tests as reference.

    vanilla ASGI: set URL instead of always "generic ASGI request"
    Starlette:
        normal request: transaction name is function name or route (depending on transaction_style setting)
        traces_sampler: always receives the raw URL as the transaction name (no matter the transaction_style setting. because we do not know more at the time the traces_sampler is called.)
        requests that end in a middleware (like 404, CORS): the functions name or the raw URL (depending on transaction_style setting)
    FastAPI
        normal request: transaction name is function name or route (depending on transaction_style setting)
        traces_sampler: always receives the raw URL as the transaction name (no matter the transaction_style setting. because we do not know more at the time the traces_sampler is called.)
        requests that end in a middleware (like 404, CORS): the functions name or the raw URL (depending on transaction_style setting)
    There used to be "generic ASGI request" transactions being created at the server startup (when a "lifespan" ASGI message was received.) Those transactions are not created anymore. (we can think of creating propper "Server was started/stopped" transactions in the future)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants