-
-
Notifications
You must be signed in to change notification settings - Fork 612
add query and argumeng log in membase #1292
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
Conversation
Review Summary by QodoAdd query and argument logging to Membase graph execution
WalkthroughsDescription• Add detailed query and argument logging in error messages • Import required utilities for logging argument conversion • Extract arguments to variable for reusability and clarity • Include query and arguments in error log output Diagramflowchart LR
A["ExecuteQueryAsync method"] -- "Extract arguments to variable" --> B["args variable"]
B -- "Convert to loggable format" --> C["KeyValue conversion"]
C -- "Include in error log" --> D["Enhanced error message"]
A -- "Add imports" --> E["KeyValue and ConvertToString utilities"]
File Changes1. src/Plugins/BotSharp.Plugin.Membase/GraphDb/MembaseGraphDb.cs
|
Code Review by Qodo
1. Query/args logged in error
|
| var argLogs = args.Select(x => (new KeyValue(x.Key, x.Value.ConvertToString())).ToString()); | ||
| _logger.LogError(ex, $"Error when executing query in {Provider} graph db. (Query: {query}, Argments: {string.Join("\r\n", argLogs)})"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1. Query/args logged in error 📘 Rule violation ⛨ Security
• The error log message includes the full query text and serialized argument values, which can contain sensitive data (PII, secrets, tokens, etc.). • This violates secure logging requirements because logs must not include sensitive data at any log level and should remain safe for auditing/debugging. • It also increases exposure of external inputs (query + parameters) without sanitization/redaction, which can leak data during incidents.
Agent Prompt
## Issue description
The error log currently includes the full graph query text and serialized argument values. This can leak sensitive data into application logs.
## Issue Context
`query` and `options.Arguments` are external inputs and may contain secrets/PII. Compliance requires logs to avoid sensitive data at any log level.
## Fix Focus Areas
- src/Plugins/BotSharp.Plugin.Membase/GraphDb/MembaseGraphDb.cs[55-56]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| var argLogs = args.Select(x => (new KeyValue(x.Key, x.Value.ConvertToString())).ToString()); | ||
| _logger.LogError(ex, $"Error when executing query in {Provider} graph db. (Query: {query}, Argments: {string.Join("\r\n", argLogs)})"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2. Catch block can throw 📘 Rule violation ⛯ Reliability
• The catch block performs serialization (ConvertToString uses JsonSerializer.Serialize) and string building, which can throw (e.g., non-serializable/cyclic argument values). • If this throws, it can mask the original exception and prevent the intended error logging + graceful return path, weakening reliability of error handling.
Agent Prompt
## Issue description
The error-handling path can throw while trying to serialize/format arguments for logging, which may mask the original exception and break the graceful return.
## Issue Context
`options.Arguments` is a `Dictionary<string, object>` and may contain arbitrary objects. Serializing arbitrary objects via `JsonSerializer.Serialize` can throw (e.g., cycles/unsupported types).
## Fix Focus Areas
- src/Plugins/BotSharp.Plugin.Membase/GraphDb/MembaseGraphDb.cs[55-56]
- src/Infrastructure/BotSharp.Abstraction/Utilities/StringExtensions.cs[125-149]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
No description provided.