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 error section in custom attributes configuration #1434

Closed
bnjjj opened this issue Jul 25, 2022 · 6 comments · Fixed by #1443
Closed

Add error section in custom attributes configuration #1434

bnjjj opened this issue Jul 25, 2022 · 6 comments · Fixed by #1443
Assignees

Comments

@bnjjj
Copy link
Contributor

bnjjj commented Jul 25, 2022

What is missing in our implementation for custom attributes and could be useful is to add an error subsection to reflect the same behavior we have in our plugin hooks. For example instead of having:

telemetry:
  metrics:
    prometheus:
      enabled: true
    common:
      attributes:
        subgraph:
          all:
            response:
              body:
              - path: .errors[0].extensions.type
                name: subgraph_error_extended_type

we could have :

telemetry:
  metrics:
    prometheus:
      enabled: true
    common:
      attributes:
        subgraph:
          all:
            errors: # Only works if it's a valid GraphQL error
              include_messages: true # Will include the error message in a message attribute
              include_locations: true
              include_paths: true
              extensions: # Include extension data
                - name: subgraph_error_extended_type # Name of the attribute
                  path: .type # JSON query path to fetch data from extensions

Originally posted by @bnjjj in #1198 (comment)

@BrynCooke
Copy link
Contributor

Q: Will the user be able to specify more than one error section? If not then error->errors.

Is there a possibility that users will want to filter only on certain paths of error. Not saying that this is a requirement, just posing the question.

@bnjjj
Copy link
Contributor Author

bnjjj commented Jul 27, 2022

No only one error. It's already written error BTW. About the filtering on specific path we could provide a path /here/is/my/path instead of just provide a bool. That could be a feature

@BrynCooke
Copy link
Contributor

So if there's only one error section, then it should be called errors as it applies to many errors.
include_message and include_path should also be plural.

@bnjjj
Copy link
Contributor Author

bnjjj commented Jul 27, 2022

Updated ! Thanks :)

@BrynCooke
Copy link
Contributor

just checking: extensions is an array? If so the example yaml needs a dash.

@bnjjj bnjjj self-assigned this Jul 27, 2022
@bnjjj
Copy link
Contributor Author

bnjjj commented Jul 27, 2022

Ok after some investigations we don't fill the path and locations field when we have an error in the subgraph http request. So for now I will omit these fields and do not add the ability to include them.
Final version:

telemetry:
  metrics:
    prometheus:
      enabled: true
    common:
      attributes:
        subgraph:
          all:
            errors: # Only works if it's a valid GraphQL error
              include_messages: true # Will include the error message in a message attribute
              extensions: # Include extension data
                - name: subgraph_error_extended_type # Name of the attribute
                  path: .type # JSON query path to fetch data from extensions

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 a pull request may close this issue.

3 participants