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

fix(tracer): avoid throwing errors in manual instrumentation when running outside of AWS Lambda #442

Merged
merged 6 commits into from
Jan 10, 2022

Conversation

dreamorosi
Copy link
Contributor

@dreamorosi dreamorosi commented Jan 7, 2022

Description of your changes

As reporded by @ijemmy during manual tests manually instrumenting a function with Tracer outside of AWS Lambda was throwing an error due to the lack of traces & context.

This was a bug since Tracer should automatically detect its running outside of Lambda and deactivate itself. The bug affected only manual instrumentation while the Middy middleware and Class decorator were unaffected.

Changes:

  • Now leveraging the setContextMissingStrategy() method from the Node.js x-ray-sdk to disable throwing errors when tracing is disabled.
  • Changed behaviour of Tracer.setSegment() & Tracer.getSegment() to respect value of Tracer. tracingEnabled
  • Now Tracer.getSegment() returns a dummy segment when tracing is disabled
  • Uniformed usage of Tracer.isTracingEnabled() over accessing attribute directly throughout class
  • Added additional unit test cases to cover new behaviours

How to verify this change

Check that all checks are successful, OR:

  1. Checkout branch locally
  2. cd packages/tracing && npm run package
  3. Init new project & install version packaged at previous step
  4. Run the lambda below (in Javascript, no TS needed) & you should not see any error
const { Tracer } = require('@aws-lambda-powertools/tracer');

const tracer = new Tracer({ serviceName: 'serverlessAirline' });

const handler = async (_event, context) => {
    const segment = tracer.getSegment(); // This is the facade segment (the one that is created by AWS Lambda)
    // Create subsegment for the function & set it as active
    const subsegment = segment.addNewSubsegment(`## ${process.env._HANDLER}`);
    tracer.setSegment(subsegment);

    // Annotate the subsegment with the cold start & serviceName
    tracer.annotateColdStart();
    tracer.addServiceNameAnnotation();

    let res;
    try {
        /* ... */
        // Add the response as metadata 
        tracer.addResponseAsMetadata(res, process.env._HANDLER);
    } catch (err) {
        // Add the error as metadata
        tracer.addErrorAsMetadata(err);
        throw err;
    } finally {
        // Close subsegment (the AWS Lambda one is closed automatically)
        subsegment.close();
        // Set back the facade segment as active again
        tracer.setSegment(segment);
    }
    console.log('ALL GOOD');
    return res;
};

handler();

Related issues, RFCs

N/A

PR status

Is this ready for review?: YES
Is it a breaking change?: NO

Checklist

  • My changes meet the tenets criteria
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in areas that should be flagged with a TODO, or hard-to-understand areas
  • My changes generate no new warnings
  • The code coverage hasn't decreased
  • I have added tests that prove my change is effective and works
  • New and existing unit tests pass locally and in Github Actions
  • Any dependent changes have been merged and published in downstream module
  • The PR title follows the conventional commit semantics

Breaking change checklist

N/A


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@github-actions github-actions bot added the bug Something isn't working label Jan 7, 2022
@dreamorosi dreamorosi added the tracer This item relates to the Tracer Utility label Jan 7, 2022
@dreamorosi dreamorosi self-assigned this Jan 7, 2022
@dreamorosi dreamorosi added this to the production-ready-release milestone Jan 7, 2022
flochaz
flochaz previously approved these changes Jan 10, 2022
Copy link
Contributor

@flochaz flochaz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

saragerion
saragerion previously approved these changes Jan 10, 2022
@saragerion
Copy link
Contributor

Left some minor comments but already looks good

@dreamorosi dreamorosi merged commit fd02acb into main Jan 10, 2022
@dreamorosi dreamorosi deleted the fix/tracer_manual_local_exec branch January 10, 2022 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tracer This item relates to the Tracer Utility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants