fix(hapi-instrumentation): close spans on errors in instrumented functions

[CptSchnitz]

Which problem is this PR solving?

• closes route span is missing when route throws an error in hapi instrumentation #572

Short description of the changes

• caught and recorded exceptions in extMethod wrapper and ended the span.
• caught and recorded exceptions in routeHandler wrapper and ended the span.
• added tests to cover both scenarios.

[codecov]

Codecov Report

Merging #589 (aa104f5) into main (e8c207b) will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #589      +/-   ##
==========================================
+ Coverage   94.94%   94.96%   +0.01%     
==========================================
  Files         200      200              
  Lines       11741    11787      +46     
  Branches     1120     1122       +2     
==========================================
+ Hits        11148    11194      +46     
  Misses        593      593              

Impacted Files	Coverage Δ
...entelemetry-instrumentation-hapi/test/hapi.test.ts	100.00% <0.00%> (ø)
...-instrumentation-hapi/test/hapi-server-ext.test.ts	100.00% <0.00%> (ø)
...emetry-instrumentation-hapi/src/instrumentation.ts	99.30% <0.00%> (+0.01%)	⬆️

[rauno56]

Thanks for yet another contribution! The implementation can be simplified - see the comments.

[rauno56]

Don't need the assignment nor await here. Just return oldHandler(...);

[CptSchnitz]

removed the unneeded assignment.
as for the await, it is needed as catch won't work on promises that are not awaited, and I also need to wait to close the span after the action was completed.
maybe i should add a test that includes an async handler?
Also, there is a handy eslint rule that covers that https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/docs/rules/return-await.md.

[rauno56]

contex.with passes return values through so the closure is not required here.
This or something similar should work:

        try {
          return api.context.with(
            api.trace.setSpan(api.context.active(), span),
            () => method(...params)
          );
        } catch (err) {
          span.recordException(err);
          span.setStatus({
            code: api.SpanStatusCode.ERROR,
            message: err.message,
          });
          throw err;
        } finally {
          span.end();
        }

Additionally, with also takes arguments to be passed to the function, so it can be simplified even further:

        try {
          return api.context.with(
            api.trace.setSpan(api.context.active(), span),
            method,
            ...params
          );
        } catch (err) {
          span.recordException(err);
          span.setStatus({
            code: api.SpanStatusCode.ERROR,
            message: err.message,
          });
          throw err;
        } finally {
          span.end();
        }

Test it out!
Docs: https://open-telemetry.github.io/opentelemetry-js/classes/_opentelemetry_context_async_hooks.asynchookscontextmanager.html#with

[CptSchnitz]

Thanks! it looks much better now.