-
- Notifications
You must be signed in to change notification settings - Fork 104
feat: add talker_graphql_logger package #443
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
base: master
Are you sure you want to change the base?
Conversation
- Add TalkerGraphQLLink for logging GraphQL operations - Support for queries, mutations, and subscriptions - Request/response duration tracking - Variable obfuscation for sensitive fields (password, token, etc.) - Configurable logging settings - Custom pen colors for different log types - Request/response/error filtering - Add TalkerKey constants for GraphQL logs - Comprehensive test coverage (20 tests) - Example and documentation
Reviewer's GuideIntroduces a new talker_graphql_logger package that provides a TalkerGraphQLLink for logging GraphQL operations (queries, mutations, subscriptions) with configurable settings, obfuscation, filtering, and custom log types, along with tests, examples, and documentation; also extends the core TalkerKey set with GraphQL-specific keys. Sequence diagram for query/mutation logging via TalkerGraphQLLinksequenceDiagram actor Developer participant GraphQLClient participant LinkChain participant TalkerGraphQLLink participant NextLink participant Server as HttpOrOtherLink participant Talker Developer->>GraphQLClient: execute(Request request) GraphQLClient->>LinkChain: request(request) LinkChain->>TalkerGraphQLLink: request(request, forward) alt logging disabled TalkerGraphQLLink-->>NextLink: forward(request) NextLink-->>Server: request(request) Server-->>NextLink: Stream<Response> NextLink-->>GraphQLClient: Stream<Response> else logging enabled and request accepted TalkerGraphQLLink->>Talker: logCustom(GraphQLRequestLog) Talker-->>Talker: store and print loop each Response from forward(request) TalkerGraphQLLink->>NextLink: forward(request) NextLink->>Server: request(request) Server-->>NextLink: Response response NextLink-->>TalkerGraphQLLink: Response response TalkerGraphQLLink->>TalkerGraphQLLink: measure durationMs alt response has errors opt errorFilter accepts TalkerGraphQLLink->>Talker: logCustom(GraphQLErrorLog) Talker-->>Talker: store and print end else successful response opt responseFilter accepts TalkerGraphQLLink->>Talker: logCustom(GraphQLResponseLog) Talker-->>Talker: store and print end end TalkerGraphQLLink-->>GraphQLClient: yield Response response end Note over TalkerGraphQLLink: On exceptions from forward(request) TalkerGraphQLLink->>TalkerGraphQLLink: compute durationMs opt errorFilter accepts TalkerGraphQLLink->>Talker: logCustom(GraphQLErrorLog(linkException)) Talker-->>Talker: store and print end TalkerGraphQLLink-->>GraphQLClient: rethrow error end Sequence diagram for subscription logging via TalkerGraphQLLinksequenceDiagram actor Developer participant GraphQLClient participant LinkChain participant TalkerGraphQLLink participant NextLink participant Server as SubscriptionLink participant Talker Developer->>GraphQLClient: subscribe(Request subscriptionRequest) GraphQLClient->>LinkChain: request(subscriptionRequest) LinkChain->>TalkerGraphQLLink: request(subscriptionRequest, forward) TalkerGraphQLLink->>TalkerGraphQLLink: _isSubscription(request) == true TalkerGraphQLLink->>Talker: logCustom(GraphQLRequestLog) Talker-->>Talker: store and print loop each subscription event TalkerGraphQLLink->>NextLink: forward(request) NextLink->>Server: start/continue subscription Server-->>NextLink: Response event NextLink-->>TalkerGraphQLLink: Response event alt event has errors TalkerGraphQLLink->>Talker: logCustom(GraphQLSubscriptionLog(eventType=error)) else data event TalkerGraphQLLink->>Talker: logCustom(GraphQLSubscriptionLog(eventType=data)) end Talker-->>Talker: store and print TalkerGraphQLLink-->>GraphQLClient: yield Response event end TalkerGraphQLLink->>Talker: logCustom(GraphQLSubscriptionLog(eventType=done)) Talker-->>Talker: store and print Note over TalkerGraphQLLink: On subscription stream error TalkerGraphQLLink->>Talker: logCustom(GraphQLErrorLog(linkException)) Talker-->>Talker: store and print TalkerGraphQLLink-->>GraphQLClient: rethrow error Class diagram for TalkerGraphQLLink and GraphQL log typesclassDiagram class TalkerGraphQLLink { +TalkerGraphQLLink(talker, settings) -Talker _talker +TalkerGraphQLLoggerSettings settings +Stream~Response~ request(Request request, NextLink forward) -bool _isSubscription(Request request) -Stream~Response~ _handleOperation(Request request, NextLink forward) -Stream~Response~ _handleSubscription(Request request, NextLink forward) -String _getOperationName(Request request) } class TalkerGraphQLLoggerSettings { +TalkerGraphQLLoggerSettings() +bool enabled +LogLevel logLevel +bool printVariables +bool printResponse +bool printQuery +int responseMaxLength +Set~String~ obfuscateFields +AnsiPen requestPen +AnsiPen responsePen +AnsiPen errorPen +AnsiPen subscriptionPen +GraphQLRequestFilter requestFilter +GraphQLResponseFilter responseFilter +GraphQLErrorFilter errorFilter +TalkerGraphQLLoggerSettings copyWith(enabled, logLevel, printVariables, printResponse, printQuery, responseMaxLength, obfuscateFields, requestPen, responsePen, errorPen, subscriptionPen, requestFilter, responseFilter, errorFilter) } class GraphQLRequestLog { +GraphQLRequestLog(message, request, settings) +Request request +TalkerGraphQLLoggerSettings settings +AnsiPen pen +String key +LogLevel logLevel +String generateTextMessage(timeFormat) } class GraphQLResponseLog { +GraphQLResponseLog(message, request, response, durationMs, settings) +Request request +Response response +int durationMs +TalkerGraphQLLoggerSettings settings +AnsiPen pen +String key +LogLevel logLevel +String generateTextMessage(timeFormat) } class GraphQLErrorLog { +GraphQLErrorLog(message, request, response, linkException, durationMs, settings) +Request request +Response response +Object linkException +int durationMs +TalkerGraphQLLoggerSettings settings +AnsiPen pen +String key +LogLevel logLevel +String generateTextMessage(timeFormat) } class GraphQLSubscriptionLog { +GraphQLSubscriptionLog(message, request, response, eventType, settings) +Request request +Response response +GraphQLSubscriptionEventType eventType +TalkerGraphQLLoggerSettings settings +AnsiPen pen +String key +LogLevel logLevel +String generateTextMessage(timeFormat) } class GraphQLSubscriptionEventType { <<enumeration>> data error done } class Talker { +Talker() +TalkerSettings settings +void logCustom(TalkerLog log) } class TalkerSettings { +void registerKeys(List~String~ keys) } class TalkerLog { +String title +AnsiPen pen +String key +LogLevel logLevel +String generateTextMessage(timeFormat) } class TalkerKey { <<abstract>> +String graphqlRequest +String graphqlResponse +String graphqlError +String graphqlSubscription } class Request { +Operation operation +Map~String,dynamic~ variables } class Response { +Map~String,dynamic~ data +List~GraphQLError~ errors } class Link { +Stream~Response~ request(Request request, NextLink forward) +Link concat(Link next) } class NextLink { +Stream~Response~ call(Request request) } class Operation { +String operationName +DocumentNode document } class GraphQLError { +String message +List~ErrorLocation~ locations +List~Object~ path } TalkerGraphQLLink --> Talker : uses TalkerGraphQLLink --> TalkerGraphQLLoggerSettings : has TalkerGraphQLLink --> Request : handles TalkerGraphQLLink --> Response : handles TalkerGraphQLLink --> Link : extends GraphQLRequestLog --> Request : logs GraphQLRequestLog --> TalkerGraphQLLoggerSettings : uses GraphQLRequestLog --> TalkerLog : extends GraphQLRequestLog --> TalkerKey : uses key GraphQLResponseLog --> Request : logs GraphQLResponseLog --> Response : logs GraphQLResponseLog --> TalkerGraphQLLoggerSettings : uses GraphQLResponseLog --> TalkerLog : extends GraphQLResponseLog --> TalkerKey : uses key GraphQLErrorLog --> Request : logs GraphQLErrorLog --> Response : logs GraphQLErrorLog --> TalkerGraphQLLoggerSettings : uses GraphQLErrorLog --> TalkerLog : extends GraphQLErrorLog --> TalkerKey : uses key GraphQLSubscriptionLog --> Request : logs GraphQLSubscriptionLog --> Response : logs GraphQLSubscriptionLog --> GraphQLSubscriptionEventType : uses GraphQLSubscriptionLog --> TalkerGraphQLLoggerSettings : uses GraphQLSubscriptionLog --> TalkerLog : extends GraphQLSubscriptionLog --> TalkerKey : uses key Talker --> TalkerSettings : has TalkerLog <|-- GraphQLRequestLog TalkerLog <|-- GraphQLResponseLog TalkerLog <|-- GraphQLErrorLog TalkerLog <|-- GraphQLSubscriptionLog Link <|-- TalkerGraphQLLink File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey there - I've reviewed your changes - here's some feedback:
- The helper for deriving the operation name is duplicated in
graphql_logs.dartandtalker_graphql_link.dart; consider centralizing this logic to a shared private function to avoid divergence and keep behavior consistent. - The
_obfuscateVariableshelper only recurses into nestedMap<String, dynamic>values and ignores lists, so sensitive fields inside arrays (e.g. a list of input objects) will not be obfuscated; consider adding list handling to obfuscate nested structures uniformly. - In
TalkerGraphQLLink.request, you rely onforward!being non-null but don't enforce it; adding an assertion or explicit error whenforwardis null would make misuse of this link easier to diagnose.
Prompt for AI Agents
Please address the comments from this code review: ## Overall Comments - The helper for deriving the operation name is duplicated in `graphql_logs.dart` and `talker_graphql_link.dart`; consider centralizing this logic to a shared private function to avoid divergence and keep behavior consistent. - The `_obfuscateVariables` helper only recurses into nested `Map<String, dynamic>` values and ignores lists, so sensitive fields inside arrays (e.g. a list of input objects) will not be obfuscated; consider adding list handling to obfuscate nested structures uniformly. - In `TalkerGraphQLLink.request`, you rely on `forward!` being non-null but don't enforce it; adding an assertion or explicit error when `forward` is null would make misuse of this link easier to diagnose. ## Individual Comments ### Comment 1 <location> `packages/talker_graphql_logger/lib/src/graphql_logs.dart:60-66` </location> <code_context> + for (final entry in variables.entries) { + if (lowerCaseFields.contains(entry.key.toLowerCase())) { + result[entry.key] = _obfuscatedValue; + } else if (entry.value is Map<String, dynamic>) { + result[entry.key] = _obfuscateVariables( + entry.value as Map<String, dynamic>, + obfuscateFields, + ); + } else { + result[entry.key] = entry.value; + } + } </code_context> <issue_to_address> **🚨 issue (security):** Variable obfuscation does not handle lists containing nested maps, which may leave sensitive values unmasked. Because the logic only recurses into `Map<String, dynamic>` values, it skips nested maps inside collections (e.g. `[{password: ...}]`). For payloads that include lists of objects, sensitive fields in those objects remain unmasked. Please update `_obfuscateVariables` to also handle `List` values by recursing into any map elements so obfuscation covers deeply nested structures. </issue_to_address> ### Comment 2 <location> `packages/talker_graphql_logger/lib/src/talker_graphql_link.dart:62` </location> <code_context> + @override + Stream<Response> request(Request request, [NextLink? forward]) { + if (!settings.enabled) { + return forward!(request); + } + </code_context> <issue_to_address> **issue (bug_risk):** The link assumes `forward` is always non-null, which can throw if used as a terminating link. In `request`, you call `forward!(request)` both in the `!settings.enabled` path and when the request is filtered. That’s safe only when this link sits in the middle of a chain; if it’s ever used as a terminating link, `forward` will be null and this will throw at runtime. To make this more robust, either assert `forward != null` at the start of `request`, or handle `forward == null` explicitly (e.g., return an error or empty stream with a clear message). </issue_to_address> ### Comment 3 <location> `packages/talker_graphql_logger/lib/src/talker_graphql_link.dart:274-283` </location> <code_context> +} + +/// Returns the operation name from the request. +String _getOperationName(Request request) { + final operationName = request.operation.operationName; + if (operationName != null && operationName.isNotEmpty) { + return operationName; + } + + final definitions = request.operation.document.definitions; + for (final definition in definitions) { + if (definition is OperationDefinitionNode) { + final name = definition.name?.value; + if (name != null) return name; + } + } + return 'Anonymous'; +} + </code_context> <issue_to_address> **suggestion:** Operation-name extraction logic is duplicated with `_getOperationName` in `graphql_logs.dart`. This implementation is effectively identical to `_getOperationName` in `graphql_logs.dart`, which risks divergence if only one is updated (for example, when supporting additional AST shapes). Please extract this into a shared helper (e.g., a private utility in a common file) and call it from both `TalkerGraphQLLink` and the logging code. Suggested implementation: ``` import 'graphql_logs.dart'; import 'graphql_operation_name.dart'; import 'talker_graphql_logger_settings.dart'; ``` ``` } ``` ``` ``` ``` getOperationName( ``` To fully implement the deduplication and avoid divergence: 1. Create a new shared helper file, for example at `packages/talker_graphql_logger/lib/src/graphql_operation_name.dart`: ```dart import 'package:gql/ast.dart'; import 'package:gql_exec/gql_exec.dart'; /// Returns the operation name from the request. String getOperationName(Request request) { final operationName = request.operation.operationName; if (operationName != null && operationName.isNotEmpty) { return operationName; } final definitions = request.operation.document.definitions; for (final definition in definitions) { if (definition is OperationDefinitionNode) { final name = definition.name?.value; if (name != null) return name; } } return 'Anonymous'; } ``` 2. In `packages/talker_graphql_logger/lib/src/graphql_logs.dart`: - Remove the existing `_getOperationName` implementation. - Add `import 'graphql_operation_name.dart';`. - Replace any calls to `_getOperationName(request)` with `getOperationName(request)`. These changes will centralize the operation-name extraction logic while keeping the call sites (`TalkerGraphQLLink` and the logging code) simple and consistent. </issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| } else if (entry.value is Map<String, dynamic>) { | ||
| result[entry.key] = _obfuscateVariables( | ||
| entry.value as Map<String, dynamic>, | ||
| obfuscateFields, | ||
| ); | ||
| } else { | ||
| result[entry.key] = entry.value; |
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.
🚨 issue (security): Variable obfuscation does not handle lists containing nested maps, which may leave sensitive values unmasked.
Because the logic only recurses into Map<String, dynamic> values, it skips nested maps inside collections (e.g. [{password: ...}]). For payloads that include lists of objects, sensitive fields in those objects remain unmasked. Please update _obfuscateVariables to also handle List values by recursing into any map elements so obfuscation covers deeply nested structures.
| @override | ||
| Stream<Response> request(Request request, [NextLink? forward]) { | ||
| if (!settings.enabled) { | ||
| return forward!(request); |
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.
issue (bug_risk): The link assumes forward is always non-null, which can throw if used as a terminating link.
In request, you call forward!(request) both in the !settings.enabled path and when the request is filtered. That’s safe only when this link sits in the middle of a chain; if it’s ever used as a terminating link, forward will be null and this will throw at runtime. To make this more robust, either assert forward != null at the start of request, or handle forward == null explicitly (e.g., return an error or empty stream with a clear message).
| String _getOperationName(Request request) { | ||
| final operationName = request.operation.operationName; | ||
| if (operationName != null && operationName.isNotEmpty) { | ||
| return operationName; | ||
| } | ||
| | ||
| final definitions = request.operation.document.definitions; | ||
| for (final definition in definitions) { | ||
| if (definition is OperationDefinitionNode) { | ||
| final name = definition.name?.value; |
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.
suggestion: Operation-name extraction logic is duplicated with _getOperationName in graphql_logs.dart.
This implementation is effectively identical to _getOperationName in graphql_logs.dart, which risks divergence if only one is updated (for example, when supporting additional AST shapes). Please extract this into a shared helper (e.g., a private utility in a common file) and call it from both TalkerGraphQLLink and the logging code.
Suggested implementation:
import 'graphql_logs.dart'; import 'graphql_operation_name.dart'; import 'talker_graphql_logger_settings.dart'; } getOperationName( To fully implement the deduplication and avoid divergence:
-
Create a new shared helper file, for example at
packages/talker_graphql_logger/lib/src/graphql_operation_name.dart:import 'package:gql/ast.dart'; import 'package:gql_exec/gql_exec.dart'; /// Returns the operation name from the request. String getOperationName(Request request) { final operationName = request.operation.operationName; if (operationName != null && operationName.isNotEmpty) { return operationName; } final definitions = request.operation.document.definitions; for (final definition in definitions) { if (definition is OperationDefinitionNode) { final name = definition.name?.value; if (name != null) return name; } } return 'Anonymous'; }
-
In
packages/talker_graphql_logger/lib/src/graphql_logs.dart:- Remove the existing
_getOperationNameimplementation. - Add
import 'graphql_operation_name.dart';. - Replace any calls to
_getOperationName(request)withgetOperationName(request).
- Remove the existing
These changes will centralize the operation-name extraction logic while keeping the call sites (TalkerGraphQLLink and the logging code) simple and consistent.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@ ## master #443 +/- ## ========================================== - Coverage 98.63% 90.97% -7.66% ========================================== Files 3 13 +10 Lines 146 266 +120 ========================================== + Hits 144 242 +98 - Misses 2 24 +22 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Thanks a lot for contributing!
Provide a description of your changes below
Summary by Sourcery
Add a new GraphQL-specific logging package integrated with Talker, including link integration, log types, settings, and documentation.
New Features:
Enhancements:
Documentation:
Tests: