[SVLS-5506] Update cloud run tags to also account for Cloud Run Functions and Namespace by nina9753 · Pull Request #30722 · DataDog/datadog-agent

nina9753 · 2024-11-04T19:15:19Z

What does this PR do?

Update cloud run origin tag to also account for Cloud Run Functions by setting the origin tag always to cloudrun
Updates tags to include namespace prefix gcr for Cloud Run Services and gcrfx for Cloud Run Functions

All spans will be tagged with:

origin
_dd.origin
_dd.datadog_sidecar_version

Duplicate tags for the overlying cloud run service for both namespaces:

gcrfx.revision_name
gcrfx.service_name
gcrfx.configuration_name
gcrfx.region
- original region tag remains
gcr.container_id
- original container_id tag remains
gcr.project_id
- original project_id tag remains

Additional tags only for Cloud Run Functions:

gcrfx.function_target
- Env var needed to determine if a service is cloud run or cloud run function
gcrfx.function_signature_type

Motivation

Based on this PR #29404
This Jira Ticket

Describe how to test/QA your changes

updated tests for cloudrun.go file in cloudrun_tests.go
and tests for gcp.go file in gcp_tests.go
PR was tested with serverless-init release candidate gcr-tag-rc1 results are shown in the image below

Possible Drawbacks / Trade-offs

we will need function_target tag to differentiate cloud run services vs cloud run functions

Additional Notes

agent-platform-auto-pr · 2024-11-04T19:44:31Z

Test changes on VM

Use this command from test-infra-definitions to manually test this PR changes on a VM:

inv create-vm --pipeline-id=48583022 --os-family=ubuntu

Note: This applies to commit 5f140e2

cit-pr-commenter · 2024-11-04T19:50:09Z

Regression Detector

Regression Detector Results

Metrics dashboard
Target profiles
Run ID: 8f341042-1844-4b87-9b41-75db838dc296

Baseline: 2e1024b
Comparison: 5f140e2
Diff

Optimization Goals: ✅ No significant changes detected

Fine details of change detection per experiment

perf	experiment	goal	Δ mean %	Δ mean % CI	trials	links
➖	basic_py_check	% cpu utilization	+2.73	[-1.29, +6.75]	1	Logs
➖	pycheck_lots_of_tags	% cpu utilization	+0.64	[-2.69, +3.97]	1	Logs
➖	otel_to_otel_logs	ingress throughput	+0.44	[-0.24, +1.12]	1	Logs
➖	file_to_blackhole_1000ms_latency	egress throughput	+0.36	[-0.12, +0.84]	1	Logs
➖	file_to_blackhole_0ms_latency	egress throughput	+0.02	[-0.45, +0.49]	1	Logs
➖	tcp_syslog_to_blackhole	ingress throughput	+0.01	[-0.04, +0.06]	1	Logs
➖	quality_gate_idle	memory utilization	+0.01	[-0.04, +0.06]	1	Logs bounds checks dashboard
➖	tcp_dd_logs_filter_exclude	ingress throughput	+0.00	[-0.01, +0.01]	1	Logs
➖	uds_dogstatsd_to_api	ingress throughput	+0.00	[-0.07, +0.07]	1	Logs
➖	file_to_blackhole_100ms_latency	egress throughput	-0.03	[-0.34, +0.29]	1	Logs
➖	file_to_blackhole_300ms_latency	egress throughput	-0.04	[-0.22, +0.15]	1	Logs
➖	file_to_blackhole_500ms_latency	egress throughput	-0.06	[-0.31, +0.18]	1	Logs
➖	file_tree	memory utilization	-0.10	[-0.22, +0.02]	1	Logs
➖	quality_gate_idle_all_features	memory utilization	-0.28	[-0.38, -0.18]	1	Logs bounds checks dashboard
➖	uds_dogstatsd_to_api_cpu	% cpu utilization	-1.46	[-2.19, -0.73]	1	Logs

Bounds Checks: ❌ Failed

perf	experiment	bounds_check_name	replicates_passed	links
❌	quality_gate_idle	memory_usage	6/10	bounds checks dashboard
❌	file_to_blackhole_0ms_latency	lost_bytes	9/10
❌	file_to_blackhole_100ms_latency	lost_bytes	9/10
✅	file_to_blackhole_0ms_latency	memory_usage	10/10
✅	file_to_blackhole_1000ms_latency	memory_usage	10/10
✅	file_to_blackhole_100ms_latency	memory_usage	10/10
✅	file_to_blackhole_300ms_latency	memory_usage	10/10
✅	file_to_blackhole_500ms_latency	memory_usage	10/10
✅	quality_gate_idle_all_features	memory_usage	10/10	bounds checks dashboard

Explanation

Confidence level: 90.00%
Effect size tolerance: |Δ mean %| ≥ 5.00%

Performance changes are noted in the perf column of each table:

✅ = significantly better comparison variant performance
❌ = significantly worse comparison variant performance
➖ = no significant change in performance

A regression test is an A/B test of target performance in a repeatable rig, where "performance" is measured as "comparison variant minus baseline variant" for an optimization goal (e.g., ingress throughput). Due to intrinsic variability in measuring that goal, we can only estimate its mean value for each experiment; we report uncertainty in that value as a 90.00% confidence interval denoted "Δ mean % CI".

For each experiment, we decide whether a change in performance is a "regression" -- a change worth investigating further -- if all of the following criteria are true:

Its estimated |Δ mean %| ≥ 5.00%, indicating the change is big enough to merit a closer look.
Its 90.00% confidence interval "Δ mean % CI" does not contain zero, indicating that if our statistical model is accurate, there is at least a 90.00% chance there is a difference in performance between baseline and comparison variants.
Its configuration does not mark it "erratic".

…n-tag-to-cloudrun

…ctions-origin-tag-to-cloudrun' into nina.rei/SVLS-5506/cloud-run-functions-origin-tag-to-cloudrun

…n-tag-to-cloudrun

jcstorms1 · 2024-11-08T18:30:37Z

 	functionTargetEnvVar    = "FUNCTION_TARGET" // exists as a cloudrunfunction env var for all runtimes except Go
 )

+const (


nit: this can probably just move to the other constants. Differentiating them with a comment is sufficient.

jcstorms1 · 2024-11-08T18:45:20Z


 // CloudRun has helper functions for getting Google Cloud Run data
 type CloudRun struct {
+	spanNamespace        string


This works well also, but I’m curious if by making everything constants as I stated earlier, you could have stuck with the original cloudRunFunctionMode bool. This way you could use the gcr constants as default and switch to the gcrfx ones if cloudRunFunctionMode=true. I think this would be easier to read as well. Can you help explain why my suggestions wouldn’t work and why you chose this approach instead?

jcstorms1 · 2024-11-08T18:45:35Z

+	cloudRunService  = "gcr."
+	cloudRunFunction = "gcrfx."


This all looks great, but I feel it might have been simpler to read if you had set these as full constants, like gcrf.function_target. Can you help me understand why appending the prefix each time you set a tag is the better approach in this case?

I can update the code with constants instead. I went with namespace prefix + tag name instead of a constant namespace tag because of the region and project_id tags. This would require a larger code refactor to check for cloudRunFunctionMode outside of cloudrun file instead of passing in the namespace prefix to the TagMap function in gcp.go. But I agree your approach is easier to read

nina9753 · 2024-11-11T14:21:46Z

/merge

dd-devflow · 2024-11-11T14:21:53Z

Devflow running: `/merge`

View all feedbacks in Devflow UI.

2024-11-11 14:21:53 UTC ℹ️ MergeQueue: pull request added to the queue

The median merge time in main is 23m.

update cloud run origin tag to also account for Cloud Run Functions

4b5b0e8

nina9753 requested a review from a team as a code owner November 4, 2024 19:15

github-actions Bot added the short review PR is simple enough to be reviewed quickly label Nov 4, 2024

nina9753 added team/serverless-azure-gcp GAS labels Nov 4, 2024

nina9753 added the changelog/no-changelog No changelog entry needed label Nov 4, 2024

update gcp tags to to have gcr or gcrfx namespace

128058a

github-actions Bot added medium review PR review might take time and removed short review PR is simple enough to be reviewed quickly labels Nov 8, 2024

nina9753 and others added 3 commits November 7, 2024 20:33

remove cloudRunFunctionMode

6c761e9

Merge branch 'main' into nina.rei/SVLS-5506/cloud-run-functions-origi…

2066609

…n-tag-to-cloudrun

Merge branch 'main' into nina.rei/SVLS-5506/cloud-run-functions-origi…

7bc40db

…n-tag-to-cloudrun

nina9753 changed the title ~~[SVLS-5506] Update cloud run origin tag to also account for Cloud Run Functions~~ [SVLS-5506] Update cloud run tags to also account for Cloud Run Functions and Namespace Nov 8, 2024

nina9753 and others added 3 commits November 8, 2024 12:08

add ContainerID and ProjectID

ca8f5d3

Merge remote-tracking branch 'origin/nina.rei/SVLS-5506/cloud-run-fun…

531a82a

…ctions-origin-tag-to-cloudrun' into nina.rei/SVLS-5506/cloud-run-functions-origin-tag-to-cloudrun

Merge branch 'main' into nina.rei/SVLS-5506/cloud-run-functions-origi…

5f140e2

…n-tag-to-cloudrun

jcstorms1 reviewed Nov 8, 2024

View reviewed changes

jcstorms1 approved these changes Nov 8, 2024

View reviewed changes

dd-mergequeue Bot merged commit 88609c3 into main Nov 11, 2024

dd-mergequeue Bot deleted the nina.rei/SVLS-5506/cloud-run-functions-origin-tag-to-cloudrun branch November 11, 2024 14:41

github-actions Bot added this to the 7.61.0 milestone Nov 11, 2024

nina9753 added the qa/done QA done before merge and regressions are covered by tests label Nov 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SVLS-5506] Update cloud run tags to also account for Cloud Run Functions and Namespace#30722

[SVLS-5506] Update cloud run tags to also account for Cloud Run Functions and Namespace#30722
dd-mergequeue[bot] merged 8 commits into
mainfrom
nina.rei/SVLS-5506/cloud-run-functions-origin-tag-to-cloudrun

nina9753 commented Nov 4, 2024 •

edited

Loading

Uh oh!

agent-platform-auto-pr Bot commented Nov 4, 2024 •

edited

Loading

Uh oh!

cit-pr-commenter Bot commented Nov 4, 2024 •

edited

Loading

Fine details of change detection per experiment

Explanation

Uh oh!

jcstorms1 Nov 8, 2024

Uh oh!

jcstorms1 Nov 8, 2024

Uh oh!

jcstorms1 Nov 8, 2024

Uh oh!

nina9753 Nov 8, 2024 •

edited

Loading

Uh oh!

nina9753 commented Nov 11, 2024

Uh oh!

dd-devflow Bot commented Nov 11, 2024 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

nina9753 commented Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR do?

Motivation

Describe how to test/QA your changes

Possible Drawbacks / Trade-offs

Additional Notes

Uh oh!

agent-platform-auto-pr Bot commented Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test changes on VM

Uh oh!

cit-pr-commenter Bot commented Nov 4, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Regression Detector

Regression Detector Results

Optimization Goals: ✅ No significant changes detected

Fine details of change detection per experiment

Bounds Checks: ❌ Failed

Explanation

Uh oh!

jcstorms1 Nov 8, 2024

Choose a reason for hiding this comment

Uh oh!

jcstorms1 Nov 8, 2024

Choose a reason for hiding this comment

Uh oh!

jcstorms1 Nov 8, 2024

Choose a reason for hiding this comment

Uh oh!

nina9753 Nov 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nina9753 commented Nov 11, 2024

Uh oh!

dd-devflow Bot commented Nov 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Devflow running: /merge

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

nina9753 commented Nov 4, 2024 •

edited

Loading

agent-platform-auto-pr Bot commented Nov 4, 2024 •

edited

Loading

cit-pr-commenter Bot commented Nov 4, 2024 •

edited

Loading

nina9753 Nov 8, 2024 •

edited

Loading

dd-devflow Bot commented Nov 11, 2024 •

edited

Loading

Devflow running: `/merge`