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 13 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: ❌ ERROR

Descriptor Linter Files Fixed Errors Elapsed time
✅ PYTHON bandit 2 0 5.22s
⚠️ PYTHON black 3 0 1 1.25s
❌ PYTHON flake8 3 1 0.64s
✅ PYTHON isort 3 0 0 0.23s
❌ PYTHON pylint 3 17 5.86s

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 12.50000% with 14 lines in your changes missing coverage. Please review.

Project coverage is 78.67%. Comparing base (82a79b2) to head (c2f4f20).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
newrelic/hooks/external_botocore.py 12.50% 14 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1227      +/-   ##
==========================================
- Coverage   81.18%   78.67%   -2.52%     
==========================================
  Files         200      200              
  Lines       21971    21987      +16     
  Branches     3482     3485       +3     
==========================================
- Hits        17837    17298     -539     
- Misses       2984     3576     +592     
+ Partials     1150     1113      -37     

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


🚨 Try these New Features:

@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?

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.

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