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 10 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.22s
✅ PYTHON black 3 0 0 1.16s
✅ PYTHON flake8 3 0 0.64s
✅ PYTHON isort 3 0 0 0.21s
✅ PYTHON pylint 3 0 5.85s

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

Project coverage is 81.18%. Comparing base (f2cdd3e) to head (456660a).

Files with missing lines Patch % Lines
newrelic/hooks/external_botocore.py 81.25% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1227      +/-   ##
==========================================
+ Coverage   81.15%   81.18%   +0.02%     
==========================================
  Files         200      200              
  Lines       21953    21969      +16     
  Branches     3479     3482       +3     
==========================================
+ Hits        17816    17835      +19     
+ Misses       2986     2983       -3     
  Partials     1151     1151              

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

Comment on lines +115 to +121
return iam.create_role(
RoleName="my-role",
AssumeRolePolicyDocument="some policy",
Path="/my-path/",
)[
"Role"
]["Arn"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
return iam.create_role(
RoleName="my-role",
AssumeRolePolicyDocument="some policy",
Path="/my-path/",
)[
"Role"
]["Arn"]
role = iam.create_role(
RoleName="my-role",
AssumeRolePolicyDocument="some policy",
Path="/my-path/",
)
return role["Role"]["Arn"]

Maybe do this to make the formatter less insufferable

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

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