Skip to content
This repository has been archived by the owner on Feb 8, 2024. It is now read-only.

Add histogram for insertion time to completion time #67

Closed
wants to merge 3 commits into from

Conversation

bretthoerner
Copy link
Contributor

@bretthoerner bretthoerner commented Feb 5, 2024

Doh, plugin-server is deployed and sends the created_at field, but I forgot to consider that hooks-api wouldn't be serializing it into the DB until deployed, and so the race of deploying hooks-api and hooks-worker left rows that couldn't be deserialized in the DB.

Making this an Option as this field isn't critical to normal operation. I go ahead and set it in hooks-api if it's missing.

@xvello was right :)

Based on created_at column now.

@xvello
Copy link
Contributor

xvello commented Feb 6, 2024

The Job struct already has the created_at field sourced from PG, I suggest using it directly. As the submission through hooks-api is synchronous, its value should be pretty close to the source-submitted metadata. I am also wondering about the confusion factor of having a created_at column in the table + a created_at metadata, and what they mean.

@bretthoerner
Copy link
Contributor Author

As the submission through hooks-api is synchronous

FWIW, my thinking was that a real end-to-end test takes into account any difficulties talking to the API (or if the API is blocked waiting on a PG connection from its pool, etc). In the end though I'm indifferent, and agree two created_at was a bad choice.

@bretthoerner bretthoerner changed the title Add proper e2e histrogram based on metadata created_at Add histogram for insertion time to completion time Feb 6, 2024
@bretthoerner
Copy link
Contributor Author

Moving this to new repo to try out actions etc. PostHog/hog-rs#5

@bretthoerner bretthoerner deleted the brett/e2e-try2 branch February 8, 2024 15:43
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants