Add _dd.gcrfx.resource_name to serverless compatibility layer for Google Cloud Functions #770
+160
−31
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
BenchmarksComparisonBenchmark execution time: 2024-12-12 19:38:59 Comparing candidate commit ac860e7 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 51 metrics, 2 unstable metrics. CandidateCandidate benchmark detailsGroup 1
Group 2
Group 3
Group 4
Group 5
Group 6
Group 7
Group 8
Group 9
Group 10
Group 11
Group 12
BaselineOmitted due to size. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #770 +/- ##
==========================================
+ Coverage 70.95% 70.99% +0.04%
==========================================
Files 313 313
Lines 45747 45870 +123
==========================================
+ Hits 32458 32566 +108
- Misses 13289 13304 +15
|
…ent-gcp-cloud-functions
…ent-gcp-cloud-functions
duncanpharvey
requested changes
Dec 12, 2024
trace-utils/src/trace_utils.rs
Outdated
| env_type: &EnvironmentType, | ||
| ) { | ||
| if let EnvironmentType::CloudFunction = env_type { | ||
| let function = span.meta.get("functionname"); |
There was a problem hiding this comment.
What do you think about passing the functionname in self.config.app_name.clone() as a parameter to the enrich_span_with_google_cloud_function_metadata method instead of fetching it from the span?
trace-utils/src/trace_utils.rs
Outdated
| ); | ||
|
|
||
| span.meta.insert( | ||
| "_dd.gcrfx.resource_name".to_string(), |
There was a problem hiding this comment.
Is there a reason this resource span tag needs to be prefixed with _dd? For Azure App Services and Functions the resource name is visible to end users (ie. not hidden by using _dd).
There was a problem hiding this comment.
That's a good point i hid the tag for now since it was only to be used to billing and matching traces to their functions. resource_name is just a combination of other tags that the customer can see like location and project_id . I can expose the tag if we think the customers would find it useful
There was a problem hiding this comment.
nevermind! After talking about it again in person, I updated the tag to exclude the _dd prefix. this will now be visible to everyone
nina9753
changed the title
Comment on lines
+525
to
+530
| let Some(region) = &mini_agent_metadata.gcp_region else { | ||
| return; | ||
| }; | ||
| let Some(project) = &mini_agent_metadata.gcp_project_id else { | ||
| return; | ||
| }; |
There was a problem hiding this comment.
Since we're short circuiting here on either the gcp_region or the gcp_project_id being empty there's a possibility that one of the span tags is not set even when the values is available. I'll approve since I think that occurrence should be rare but just wanted to point that out in case it's not what was intended!
There was a problem hiding this comment.
let Some().... else {return;}; should check/handle the error if the value is empty and should return before adding the resource_name span tag.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Adds
gcrfx.resource_namespan tag to cloud run services needed for trace mapping by creating a unified tag of the necessary fields i.eprojects/{}/locations/{}/functions/{}to the top level span onlyUpdates tests to look for the new
gcrfx.resource_namespan tagMotivation
https://datadoghq.atlassian.net/browse/SVLS-6008
Additional Notes
relates to this PR #31434
billing is now done in this PR