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

feat: add Server-Timing header with JWT duration #2937

Merged
merged 2 commits into from
Sep 15, 2023

Conversation

taimoorzaeem
Copy link
Collaborator

Part of #2771.

@taimoorzaeem taimoorzaeem force-pushed the server-timing branch 2 times, most recently from 2c04650 to 60fe59c Compare September 12, 2023 14:28
@steve-chavez
Copy link
Member

@taimoorzaeem Interesting feature! So I've been playing a bit with it and I've noticed that:

$ PGRST_JWT_SECRET="reallyreallyreallyreallyverysafe" PGRST_DB_PLAN_ENABLED="true" postgrest-with-postgresql-15 postgrest-run
export JWT="eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJleHAiOjk5OTk5OTk5OTksInJvbGUiOiJwb3N0Z3Jlc3RfdGVzdF9hdXRob3IiLCJpZCI6Impkb2UiLCJhdWQiOiJ5b3VyYXVkaW
VuY2UifQ.fJ4tLKSmolWGWehWN20qiU9dMO-WY0RI2VvacL7-ZGo"

$ curl 'localhost:3000/authors_only' -H "Authorization: Bearer $JWT" -D -
HTTP/1.1 200 OK
Server-Timing: jwt;dur=335.1

$ curl 'localhost:3000/authors_only' -H "Authorization: Bearer $JWT" -D -
HTTP/1.1 200 OK
Server-Timing: jwt;dur=147.7

$ curl 'localhost:3000/authors_only' -H "Authorization: Bearer $JWT" -D -
HTTP/1.1 200 OK
Server-Timing: jwt;dur=145.9

$ curl 'localhost:3000/authors_only' -H "Authorization: Bearer $JWT" -D -
Server-Timing: jwt;dur=146.2

# restarting the postgrest server

$ curl 'localhost:3000/authors_only' -H "Authorization: Bearer $JWT" -D -
HTTP/1.1 200 OK
Server-Timing: jwt;dur=327.1

$ curl 'localhost:3000/authors_only' -H "Authorization: Bearer $JWT" -D -
HTTP/1.1 200 OK
Server-Timing: jwt;dur=141.3

$ curl 'localhost:3000/authors_only' -H "Authorization: Bearer $JWT" -D -
HTTP/1.1 200 OK
Server-Timing: jwt;dur=143.9

$ curl 'localhost:3000/authors_only' -H "Authorization: Bearer $JWT" -D -
HTTP/1.1 200 OK
Server-Timing: jwt;dur=142.5

The first jwt decode always takes longer than the subsequent ones. Is this because of laziness? Related: #2450

@steve-chavez
Copy link
Member

@taimoorzaeem Also please rebase. Sorry, I wanted to do the refactoring for completing all the other durations (#2771 (comment)) but I got carried away and did other changes.

@steve-chavez
Copy link
Member

Strange that the idempotence test fails. Maybe some mutations on the new tests are not rolled back (which is the default for all tests configDbTxRollbackAll = True).

src/PostgREST/Auth.hs Outdated Show resolved Hide resolved
@taimoorzaeem
Copy link
Collaborator Author

taimoorzaeem commented Sep 13, 2023

The first jwt decode always takes longer than the subsequent ones. Is this because of laziness? Related: #2450

Hmm, I'll get into it. Would that be okay to test this using strict evaluation using seq? I am not sure because AFAIK GHC optimizes code much better with laziness than with strict evaluation. So maybe getting rid of laziness might not help us.

@steve-chavez
Copy link
Member

Would that be okay to test this using strict evaluation using seq? I am not sure because AFAIK GHC optimizes code much better with laziness than with strict evaluation. So maybe getting rid of laziness might not help us.

Yeah, not necessary to be strict. I mostly wanted to check if it's really because of laziness and why would it take more than double the time the first time. On #2450, it's clear it takes a while because the schema cache loading is an expensive process. On this case I don't know what initialization takes place.

@taimoorzaeem
Copy link
Collaborator Author

Strange that the idempotence test fails. Maybe some mutations on the new tests are not rolled back (which is the default for all tests configDbTxRollbackAll = True).

I think this is more than just mutations not being rollbacked. For instance:

test/spec/Feature/Query/QuerySpec.hs:64:34: 
  1) Feature.Query.QuerySpec, Filtering response, matches nulls using not operator
       body mismatch:
         expected: "[{\"a\":\"1\",\"b\":\"0\"},{\"a\":\"2\",\"b\":\"0\"}]"
         but got:  "[{\"a\":\"1\",\"b\":\"1\"}, \n {\"a\":\"2\",\"b\":\"1\"}]"

Where is the \n coming from? So yeah, strange indeed.

postgrest.cabal Outdated Show resolved Hide resolved
@wolfgangwalther
Copy link
Member

The first jwt decode always takes longer than the subsequent ones. Is this because of laziness?

I just tested and I observe the same thing. I wondered whether that was only the case with the same JWT token or also with different ones: It's the same, even when using different JWTs between requests. The first one will always be significantly slower.

Any tests for the jwt caching feature will need to take this into account. They'd basically need a "warmup" before doing any testing.

@steve-chavez steve-chavez merged commit d64b71c into PostgREST:main Sep 15, 2023
@steve-chavez
Copy link
Member

@taimoorzaeem Btw, could we make the JWT evaluation stricter with bang patterns?

Pros of doing this:

  • Will not confuse users when reading Server-Timing durations.
  • Make JWT caching easier to test.

@taimoorzaeem
Copy link
Collaborator Author

@taimoorzaeem Btw, could we make the JWT evaluation stricter with bang patterns?

Hmm, I tried BangPatterns but it made no difference to the duration.

@taimoorzaeem taimoorzaeem deleted the server-timing branch September 27, 2023 17:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants