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.

Sign up for GitHub

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

Jump to bottom

Add ability to enable/disable the SDK (#26) #119

Merged
merged 8 commits into from
Feb 6, 2019
Merged

Add ability to enable/disable the SDK (#26) #119

merged 8 commits into from
Feb 6, 2019

Conversation

chanchiem
Copy link
Contributor

Added the ability to enable/disable the SDK upon configuration. This prevents application unit tests that originally relied on segments to be generated to no longer throw SegmentNotFound exceptions if a unit test tests a method that relies on a previously generated Segment.

Issue #, if available:
#26

Description of changes:

  • Added SDKConfig to hold SDK-wide configuration information. In our case, whether it's enabled/disabled is kept track of. This is used by the Patcher, Recorder, and the Lambda Context to change behaviors if it's disabled.
  • For the case of the Patcher, patching does not patch any methods.
  • For the Recorder, all segment creation automatically creates DummySegments and therefore not send Segments and subsequent Subsegments to the Daemon. Using DummySegments ensures that downstream methods calls on the segment/subsegment entities don't throw exceptions.
  • To deal with the Capture decorator, a Dummy parent segment is automatically generated to mimic middleware segment generation.

Testing

  • Core Patcher module should not import modules when SDK is disabled
  • Each specific middleware (Flask, Django, Aiohttp) is tested to ensure that they produce DummySegments and dont send anything to the emitter.
  • Recorder should produce DummySegments and DummySubsegments
  • SDKConfig should take Environment Variables over hardcoded configuration.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@chanchiem
Add ability to enable/disable the SDK (aws#26)
6de2714 
* Disabling the SDK does the following:
    + Ext module patch methods no longer patch.
    + Any call to BeginSegment() automatically generates a DummySegment instead and will not be sent
      to the Daemon, and consequently, since DummySegments are generated, any generated subsegments
      automatically become DummySubsegments.
    + For Lambda, when Subsegments are created, they automatically are converted to DummySubsegments
* Disabling/Enabling is done through the configure() method of the recorder. It may also be set through
  an environment variable "AWS_XRAY_ENABLED".
@chanchiem chanchiem requested a review from haotianw465 January 17, 2019 00:42
haotianw465
haotianw465 suggested changes Jan 19, 2019
View reviewed changes
Copy link
Contributor

@haotianw465 haotianw465 left a comment

Choose a reason for hiding this comment

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

Default sampler code also need to be changed so it becomes no-op if the SDK is disabled. Right now the middleware code still trigger sampling decisions which creates extra pollers.

aws_xray_sdk/core/lambda_launcher.py Outdated Show resolved Hide resolved
aws_xray_sdk/core/lambda_launcher.py Outdated Show resolved Hide resolved
aws_xray_sdk/core/recorder.py Outdated Show resolved Hide resolved
aws_xray_sdk/sdk_config.py Outdated Show resolved Hide resolved
@chanchiem
Add ability to enable/disable the SDK (aws#26)
e960164 
Revision 2

Changes:
    - SDKConfig now built on top of recorder. SDK-level configuration should all be done
      through SDKConfig.
    - Renamed the SDK Enable environment variable to AWS_XRAY_SDK_ENABLED
    - Disabling the SDK now causes LambdaContext to set sampling decision of facade segment to False.
      Previously, it would force all subsegments to be generated as dummy subsegments.
    - SDKConfig.set_sdk_enabled(value) method defaults to True and throws an exception
      if value is not of type boolean.
@chanchiem
Copy link
Contributor Author

Second revision added.

Changes:
- SDKConfig now built on top of recorder. SDK-level configuration should all be done
through SDKConfig.
- Renamed the SDK Enable environment variable to AWS_XRAY_SDK_ENABLED
- Disabling the SDK now causes LambdaContext to set sampling decision of facade segment to False.
Previously, it would force all subsegments to be generated as dummy subsegments.
- SDKConfig.set_sdk_enabled(value) method defaults to True and throws an exception
if value is not of type boolean.

@chanchiem
Copy link
Contributor Author

Third revision added:

Changes:

  • Removed enable flag from recorder. Now it checks the global SDK configuration to determine if the SDK is disabled.
  • Global SDK configuration no longer modifies the recorder.
  • Tests modified to reflect changes.

@chanchiem
Add ability to enable/disable the SDK (aws#26)
682703e 
Revision 3

Changes:
        - Removed enable flag from recorder. Now it checks the
        global SDK configuration to determine if the SDK is disabled.
        Global SDK configuration no longer modifies the recorder.
        - Tests modified to reflect change.
haotianw465
haotianw465 suggested changes Jan 29, 2019
View reviewed changes
aws_xray_sdk/core/lambda_launcher.py Outdated Show resolved Hide resolved
aws_xray_sdk/core/patcher.py Outdated Show resolved Hide resolved
aws_xray_sdk/core/patcher.py Show resolved Hide resolved
aws_xray_sdk/core/recorder.py Outdated Show resolved Hide resolved
aws_xray_sdk/core/recorder.py Outdated Show resolved Hide resolved
@chanchiem
Merge remote-tracking branch 'origin/master' into chiem_disabled
bcc9857
@chanchiem
Add ability to enable/disable the SDK (aws#26)
322792a 
Revision 4

Changes:
        - Refactored multiple modules; instead of import aws_xray_sdk to get the global_sdk_config,
        direct import using from... import.
        - Removed unused XRAY_ENABLED_KEY variable from recorder. It wasn't being used.
        - Added debug log entry to the patcher to inform the customer that patching is disabled
        when the SDK is disabled.
@chanchiem
Copy link
Contributor Author

Fourth Revision Added:

Changes:

  • Refactored multiple modules; instead of import aws_xray_sdk to get the global_sdk_config, direct import using from... import.
  • Removed unused XRAY_ENABLED_KEY variable from recorder.
  • Added debug log entry to the patcher to inform the customer that patching is disabled when the SDK is disabled.

haotianw465
haotianw465 reviewed Feb 5, 2019
View reviewed changes
aws_xray_sdk/core/sampling/sampler.py Outdated Show resolved Hide resolved
aws_xray_sdk/sdk_config.py Outdated Show resolved Hide resolved
@chanchiem
Add ability to enable/disable the SDK (aws#26)
e443268 
Revision 5

Changes:
    - Refactored sampler to use "from ... import ..." for the global sdk config module.
    - global sdk config logs and defaults to true instead of throwing an exception
    when an invalid parameter is passed into the method.
@chanchiem
Copy link
Contributor Author

Fifth revision added:

Changes:

  • Refactored sampler to use "from ... import ..." for the global sdk config module.
  • Global sdk config logs and defaults to true instead of throwing an exception when an invalid parameter is passed into the method.
  • Removed unused key in recorder unit test.

@haotianw465 haotianw465 merged commit d081441 into aws:master Feb 6, 2019
@chanchiem chanchiem mentioned this pull request Feb 6, 2019
Closed
@chanchiem chanchiem deleted the rundisable branch February 8, 2019 00:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haotianw465 haotianw465 haotianw465 approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@chanchiem @haotianw465