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: track completion request origin #61

Merged
merged 2 commits into from
Jan 25, 2024
Merged

Conversation

louisgv
Copy link
Contributor

@louisgv louisgv commented Jan 25, 2024

Details

Code of Conduct

  • I agree to follow this project's Code of Conduct
  • I agree to license this contribution under the MIT LICENSE
  • I checked the current PR for duplication.

Copy link
Collaborator

@sambarnes sambarnes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

payload: CompletionPayload,
):
model_path = get_model_path(payload.model)
logger.info(
"Received completion request", extra={"model": str(model_path)}
"Received completion request",
extra={
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC at opensea we did something that set this context globally for a request so all logs in the call stack used it 🤔 but that's just something in the back of my mind, we can do it later if necessary

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of this we'd probably get for free if we transition to data dog tracing anyhow so no point worrying about it now I spose

"Received completion request",
extra={
"model": str(model_path),
"user-agent": request.headers.get("user-agent"),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forget if this key needs to be snake_case for datadog to pick it up

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works :D

Screenshot 2024-01-26 at 12 54 22 AM

Copy link
Contributor

@alexanderatallah alexanderatallah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved aside from sam's potential comment about snake case

@louisgv louisgv merged commit 65b14fb into main Jan 25, 2024
3 checks passed
@louisgv louisgv deleted the lab/track-completion-origin branch January 25, 2024 18:01
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 this pull request may close these issues.

3 participants