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

Add utilities to dump Copilot traffic. #664

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

blkt
Copy link
Contributor

@blkt blkt commented Jan 20, 2025

This change adds a utility routine to dump raw data tapped from the wire to multiple files in a temporary directory under the path specified via CODEGATE_DUMP_DIR.

No instrumentation is performed if CODEGATE_DUMP_DIR is None. The temporary directory is not deleted when the process exits.

NOTE: the utility does not ensure the folder specified via CODEGATE_DUMP_DIR is writable, which might cause user-visible failures at startup.

@jhrozek
Copy link
Contributor

jhrozek commented Jan 24, 2025

@blkt great, thank you, can you take this out of draft so we can ack and merge? I'll add a similar utility to the cursor provider, just an inline copy for now, and once we refactor we can merge the two.

@blkt blkt marked this pull request as ready for review January 24, 2025 09:08
This change adds a utility routine to dump raw data tapped from the
wire to multiple files in a temporary directory under the path
specified via `CODEGATE_DUMP_DIR`.

No instrumentation is performed if `CODEGATE_DUMP_DIR` is `None`. The
temporary directory is not deleted when the process exits.

NOTE: the utility does not ensure the folder specified via
`CODEGATE_DUMP_DIR` is writable, which might cause user-visible
failures at startup.

Co-authored-by: Radoslav Dimitrov <radoslav@stacklok.com>
@blkt blkt force-pushed the chore/add-dump-request-utilities branch from e3bbc63 to e432e54 Compare January 24, 2025 13:53
@@ -1,6 +1,10 @@
import asyncio
import contextlib
Copy link
Contributor

Choose a reason for hiding this comment

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

do we use this import?

Copy link
Contributor

@jhrozek jhrozek left a comment

Choose a reason for hiding this comment

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

I'm not sure about contextlib, but otherwise looks good

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.

2 participants