refactor(telemetry): add functionality to @withTelemetryContext #5816
Conversation
nkomonen-amazon
force-pushed
the
addErrorContext
branch
4 times, most recently
from
b3f2ad9 to
5b1ee06
Compare
nkomonen-amazon
force-pushed
the
addErrorContext
branch
from
5b1ee06 to
5efed26
Compare
|
/runIntegrationTests |
nkomonen-amazon
force-pushed
the
addErrorContext
branch
from
5efed26 to
631b684
Compare
|
@justinmk3 can you give this a look when you have time |
| ) | ||
| }) | ||
|
|
||
| const customWithTelemetryContext = withTelemetryContextFactory({ class: 'TestCustomContext', emit: true }) |
There was a problem hiding this comment.
Modules can define their own predefined fields. That's more explicit, and avoids this extra layer (we already have too many layers). And avoids these extra tests.
There was a problem hiding this comment.
Makes sense, removed
|
Manually annotating the "code path" can be useful in specific, but it is not a sustainable approach in general. Can we evaluate something like https://github.com/felixge/node-stack-trace to inspect the actual stacktrace instead? |
|
@justinmk3 agreed that something more automatic would be better. A possible issue with stack traces is due to the bundling, there may be a way around that but it doesn't look trivial. |
Ah right. I think we can (and should) fix that with sourcemaps, and e.g. webpack config: https://stackoverflow.com/a/58448086/152142 To unblock this, can we remove |
The withTelemetryContext() decorator adds context to telemetry but not thrown errors. Solution: Adding this decorator to a method will add context to any thrown exceptions. This is helpful in telemetry as it will provide information about the caller. Signed-off-by: nkomonen-amazon <[email protected]>
Now the @withTelemetryContext decorator can have a method emit when called and will also add the `source` field automatically. By default it does not emit. Signed-off-by: nkomonen-amazon <[email protected]>
Signed-off-by: nkomonen-amazon <[email protected]>
Signed-off-by: nkomonen-amazon <[email protected]>
- Simplify tests - Fix typos Signed-off-by: nkomonen-amazon <[email protected]>
Signed-off-by: nkomonen-amazon <[email protected]>
It can just be done explicitly by anything that needs it Signed-off-by: nkomonen-amazon <[email protected]>
nkomonen-amazon
force-pushed
the
addErrorContext
branch
from
631b684 to
c3d68bf
Compare
justinmk3
left a comment
justinmk3
left a comment
There was a problem hiding this comment.
We should find a way to solve this problem that doesn't require explicit annotations. Can merge this for now though.
3 participants
See each commit for the specific change. But this has multiple improvements to the
@withTelemetryContextdecorator.The main change to note:
@withTelemetryContextdecorator to a method can wrap thrown errors with context about that function. This helps us to build some sort of stack trace in thereasonDescof our telemetry when errors are thrown.License: I confirm that my contribution is made under the terms of the Apache 2.0 license.