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

[Feature Request] Add Support for Query String Transformation in QueryHook #1104

Closed

Conversation

takaaa220
Copy link
Contributor

@takaaa220 takaaa220 commented Jan 8, 2025

Summary

This PR introduces a new feature to QueryHook of bundebug that allows users to customize how query strings are transformed before being logged. By providing a string -> string function through the new WithQueryConvertFunc option, users can truncate long queries, mask sensitive data, or apply other transformations tailored to their needs.

Changes

  • Added queryConvertFunc Field:
    • A new field in QueryHook to hold a custom function for transforming query strings.
  • Added WithQueryConvertFunc Option:
    • A new option function to allow users to set the queryConvertFunc field in QueryHook.

Purpose

Question

  • How unit tests should I add for this change? i am sorry, but i am not sure...
  • Do you need this feature? useful? If this feature is deemed unnecessary, please feel free to close this Pull Request without hesitation :)

@j2gg0s
Copy link
Collaborator

j2gg0s commented Jan 9, 2025

Thank you very much for your help.

From my personal experience, I might prefer the output to be

[bun]  00:07:34.171   INSERT               44.477ms  INSERT INTO "files" ("name", "content", "content_type", "size", "created_at", "updated_at", "id") VALUES (?, ?, ?, ?, ?, ?, ?) picture.jpg, \x255044462d312e..., hex, 11234, "2024-11-12 12:02:02T", "2024-11-12 12:02:02T", 123456

rather than

[bun]  00:07:34.171   INSERT               44.477ms  INSERT INTO "files" ("name", "content", "content_type", "size", "created_at", "updated_at", "id") VALUES ('picture.jpg', '\x255044462d312e340a25d3ebe9e10......

Specific args are restricted in the output, while others remain unaffected.

Perhaps we should pass the QueryEvent to the user,
allowing them greater flexibility to customize the content of the log output.
The function takes a QueryEvent as input, returns multiple any values, and outputs them to Fprintln.

@j2gg0s
Copy link
Collaborator

j2gg0s commented Jan 9, 2025

extra/bundebug looks like an example for demonstration purposes and doesn’t require unit tests.

Also, there’s no need to add an example for extra/bundebug, the relevant usage can be directly included as an option in the code.

@takaaa220
Copy link
Contributor Author

takaaa220 commented Jan 10, 2025

@j2gg0s
Thank you for your thoughtful feedback.

In fact, I initially considered passing the QueryEvent to the user to allow greater flexibility in customizing the log output. However, I decided against it because I felt that users who need access to the QueryEvent should create their own QueryHook instead. Based on this reasoning, I implemented a simpler version that only passes the query string.

After reflecting on this feature, I’ve realized that it might not be as useful to users as I initially thought. Therefore, I’ve decided to close this Pull Request.

Thank you again for taking the time to review and provide your insights.

@takaaa220 takaaa220 closed this Jan 10, 2025
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