opentelemetry-js icon indicating copy to clipboard operation
opentelemetry-js copied to clipboard

fix(instrumentation-http): do not overwrite span error status if already set

Open luismiramirez opened this issue 2 years ago • 2 comments

Which problem is this PR solving?

If a span has already set its status as Error (2), it shouldn't be overwritten as this may result in its status changing to Ok even with exceptions recorded in the events attribute.

Short description of the changes

  • Small refactor to make setSpanWithError() accept a new optional parameter (statusCode) to set the desired status if the default error one is not wanted.
  • Check if the given span has already set the error status before trying to overwrite the status.

Type of change

Please delete options that are not relevant.

  • [x] Bug fix (non-breaking change which fixes an issue)
  • [ ] New feature (non-breaking change which adds functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [ ] This change requires a documentation update

How Has This Been Tested?

Unit tests

Checklist:

  • [x] Followed the style guidelines of this project
  • [x] Unit tests have been added
  • [x] Documentation has been updated

luismiramirez avatar Aug 03 '22 10:08 luismiramirez

There are two different Span types that fail the build. A bit of help/guidance here would be much appreciated.

luismiramirez avatar Aug 03 '22 12:08 luismiramirez

Force pushing and changing history makes it really hard to see what is changed on each iteration. Please try to use regular merges

dyladan avatar Aug 09 '22 13:08 dyladan

I'll work on this shortly. I am closing the PR to keep the repo clean.

luismiramirez avatar Aug 19 '22 08:08 luismiramirez