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

Add attrs for aws lambda entity #1227

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open

Conversation

hmstepanek
Copy link
Contributor

@hmstepanek hmstepanek commented Oct 7, 2024

Overview

Add attrs for aws lambda entity linking.

The agent spec says:

The following attributes SHOULD be added to Spans created by the invoke method in the SDK.

  • cloud.platform: the string aws_lambda
  • cloud.resource_id: the ARN for the Lambda, or an alias or version, if it can be determined.

If the ARN cannot be determined, the cloud.resource_id attribute SHOULD NOT be set.

Copy link

github-actions bot commented Oct 7, 2024

🦙 MegaLinter status: ✅ SUCCESS

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON bandit 2 0 5.7s
✅ PYTHON black 3 0 0 1.34s
✅ PYTHON flake8 3 0 0.73s
✅ PYTHON isort 3 0 0 0.32s
✅ PYTHON pylint 3 0 6.46s

See detailed report in MegaLinter reports
Set VALIDATE_ALL_CODEBASE: true in mega-linter.yml to validate all sources, not only the diff

MegaLinter is graciously provided by OX Security

@mergify mergify bot added the tests-failing label Oct 7, 2024
@codecov-commenter
Copy link

codecov-commenter commented Oct 7, 2024

Codecov Report

Attention: Patch coverage is 93.75000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 81.06%. Comparing base (4853f32) to head (ec94f91).

Files with missing lines Patch % Lines
newrelic/hooks/external_botocore.py 93.75% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1227      +/-   ##
==========================================
- Coverage   81.07%   81.06%   -0.02%     
==========================================
  Files         200      200              
  Lines       22088    22118      +30     
  Branches     3508     3515       +7     
==========================================
+ Hits        17908    17929      +21     
- Misses       3017     3023       +6     
- Partials     1163     1166       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hmstepanek hmstepanek marked this pull request as ready for review November 6, 2024 23:45
@hmstepanek hmstepanek requested a review from a team as a code owner November 6, 2024 23:45

bound_args = bind_args(wrapped, args, kwargs)
arn = bound_args["kwargs"].get("FunctionName")
if arn and hasattr(instance, "meta") and hasattr(instance.meta, "events"):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just an object that I found that is also available inside the make_request. It's possible there is something better to attach this to but it was hard to find something that was accessible in both spots. I didn't want to attach it to the transaction as I thought there might be a conflict with that in async cases.

Comment on lines 973 to 976
lambda_arn = getattr(instance._event_emitter, "_nr_lambda_arn", None)
if lambda_arn:
trace._add_agent_attribute("cloud.platform", "aws_lambda")
trace._add_agent_attribute("cloud.resource_id", lambda_arn)
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we confident this object isn't reused for future requests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've updated this to use a different object that is generated for each request.

tests/external_botocore/test_boto3_lambda.py Outdated Show resolved Hide resolved
@@ -303,6 +303,7 @@ deps =
external_botocore-botocore128: botocore<1.29
external_botocore-botocore0125: botocore<1.26
external_botocore: moto
external_botocore: docker
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the docker dependency for? Lambda/moto?

Copy link
Contributor Author

@hmstepanek hmstepanek Nov 22, 2024

Choose a reason for hiding this comment

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

Yes it's imported inside the lambda invoke in moto.

mergify bot and others added 7 commits November 25, 2024 18:41
Co-authored-by: Timothy Pansino <11214426+TimPansino@users.noreply.github.com>
These are regenerated for each request as opposed to the events obj that persists across
multiple requests. Unfortunately, we have to pass the arn from api_params to the
request_dict because:
1. api_params has validation key checks that exclude unknown params upon generating the
   request_dict
2. the arn is not available after api_params are emitted because emit drops the param
   that contains the arn

So...we capture the arn before it is lost when the api_params are emitted and pop it off
before generating the request dict and put it onto the output request dict to be popped
off later by the make request method and added to the trace to link the entity.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants