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

Use span's recordException to record Errors #9

Merged
merged 1 commit into from
May 2, 2024
Merged

Conversation

MichaelDoyle
Copy link
Member

@MichaelDoyle MichaelDoyle commented May 2, 2024

This follows the recommendations of Open Telemetry and will solve two outstanding issues for us.

  1. Currently stacktraces are not appearing in Cloud Trace.
  2. We need a nice normalized message and stacktrace for the dev ui.

Genkit Trace Store (AFTER)

"status": {
  "code": 2,
  "message": "Error: Got an error!"
 },
"timeEvents": {
  "timeEvent": [
    {
      "time": 1714531849038.1628,
      "annotation": {
        "attributes": {
          "exception.type": "Error",
          "exception.message": "Got an error!",
          "exception.stacktrace": "Error: Got an error!\n    at /Users/michaeldoyle/repos/src/genkit/js/samples/dev-ui-gallery/lib/main/flows.js:148:23\n    at Context.<anonymous> (/Users/michaeldoyle/repos/src/genkit/js/flow/lib/context.js:71:27)\n    at Generator.next (<anonymous>)\n    at /Users/michaeldoyle/repos/src/genkit/js/flow/lib/context.js:36:61\n    at new Promise (<anonymous>)\n    at __async (/Users/michaeldoyle/repos/src/genkit/js/flow/lib/context.js:20:10)\n    at Context.memoize (/Users/michaeldoyle/repos/src/genkit/js/flow/lib/context.js:67:12)\n    at Context.<anonymous> (/Users/michaeldoyle/repos/src/genkit/js/flow/lib/context.js:105:58)\n    at Generator.next (<anonymous>)\n    at /Users/michaeldoyle/repos/src/genkit/js/flow/lib/context.js:36:61"
        },
        "description": "exception"
      }
    }
  ]
}

Cloud Trace (BEFORE)

Caught Errors

image

Uncaught Errors

image

Cloud Trace (AFTER)

Caught Errors

image

Uncaught Errors

image

@pavelgj
Copy link
Collaborator

pavelgj commented May 2, 2024

Oh, I just noticed the screenshots... this allows errors to be rendered nicely in Cloud Tracing?

@MichaelDoyle
Copy link
Member Author

MichaelDoyle commented May 2, 2024

Oh, I just noticed the screenshots... this allows errors to be rendered nicely in Cloud Tracing?

It does. 😄 I talked to Kman yesterday about recording exceptions and apparently this was the expected behavior, but maybe got lost in the shuffle somewhere. I am going to run this by him just to make sure it matches his expectations.

One thing I am not 💯 on is that telemetry also has a concept of recording exceptions (via logging); not sure if there is any interplay between that and the trace data.

@MichaelDoyle MichaelDoyle merged commit 4978e5d into main May 2, 2024
4 checks passed
@MichaelDoyle MichaelDoyle deleted the record-errors branch May 2, 2024 18:23
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.

4 participants