From 397518aeb1cdae6f5db4a0e953696ad18992301e Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 20 Feb 2025 19:03:14 +0000 Subject: [PATCH 1/2] fix(configurations): correctly parse `payload_tagging_services` [backport 3.0] (#12441) Backport 6e58426ad27df34a738a1cd78822a5e67a155393 from #12180 to 3.0. Previously this: ```python "payload_tagging_services": set( os.getenv("DD_TRACE_CLOUD_PAYLOAD_TAGGING_SERVICES", default={"s3", "sns", "sqs", "kinesis", "eventbridge"}) ), ``` would require users to do `config.botocore["payload_tagging_services"].add("SERVICE_HERE")` as `os.getenv` just returns `string`. If someone were to do: ```powershell $env:DD_TRACE_CLOUD_PAYLOAD_TAGGING_SERVICES = "foo,bar" ``` it would be converted into the `set` of `{'o', 'b', 'r', 'f', ',', 'a'}` This correctly parses it as a set of `{"foo", "bar"}` while also stripping any potential whitespace. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) Co-authored-by: Steven Bouwkamp Co-authored-by: Joey Zhao <5253430+joeyzhao2018@users.noreply.github.com> --- ddtrace/contrib/internal/botocore/patch.py | 5 +- ...oad_tagging_services-794cdf0f340975e6.yaml | 4 + tests/contrib/botocore/test.py | 52 ++++ ...oreTest.test_dynamodb_payload_tagging.json | 275 ++++++++++++++++++ 4 files changed, 335 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/correctly-parse-payload_tagging_services-794cdf0f340975e6.yaml create mode 100644 tests/snapshots/tests.contrib.botocore.test.BotocoreTest.test_dynamodb_payload_tagging.json diff --git a/ddtrace/contrib/internal/botocore/patch.py b/ddtrace/contrib/internal/botocore/patch.py index 61353e7b34e..bd1f815032a 100644 --- a/ddtrace/contrib/internal/botocore/patch.py +++ b/ddtrace/contrib/internal/botocore/patch.py @@ -113,7 +113,10 @@ def _load_dynamodb_primary_key_names_for_tables() -> Dict[str, Set[str]]: os.getenv("DD_TRACE_CLOUD_PAYLOAD_TAGGING_MAX_TAGS", 758) ), # RFC defined default limit - spans are limited past 1000 "payload_tagging_services": set( - os.getenv("DD_TRACE_CLOUD_PAYLOAD_TAGGING_SERVICES", default={"s3", "sns", "sqs", "kinesis", "eventbridge"}) + service.strip() + for service in os.getenv("DD_TRACE_CLOUD_PAYLOAD_TAGGING_SERVICES", "s3,sns,sqs,kinesis,eventbridge").split( + "," + ) ), }, ) diff --git a/releasenotes/notes/correctly-parse-payload_tagging_services-794cdf0f340975e6.yaml b/releasenotes/notes/correctly-parse-payload_tagging_services-794cdf0f340975e6.yaml new file mode 100644 index 00000000000..b6e4c5758cd --- /dev/null +++ b/releasenotes/notes/correctly-parse-payload_tagging_services-794cdf0f340975e6.yaml @@ -0,0 +1,4 @@ +fixes: + - | + configurations: This fix resolves an issue where DD_TRACE_CLOUD_PAYLOAD_TAGGING_SERVICES env variable was not parsed correctly + diff --git a/tests/contrib/botocore/test.py b/tests/contrib/botocore/test.py index cb1a06bec1c..3375a21c407 100644 --- a/tests/contrib/botocore/test.py +++ b/tests/contrib/botocore/test.py @@ -64,6 +64,7 @@ "meta.aws.response.body.HTTPHeaders.content-length", "meta.aws.response.body.HTTPHeaders.x-amzn-requestid", "meta.error.stack", + "meta.aws.response.body.HTTPHeaders.x-amz-crc32", ] @@ -4093,6 +4094,57 @@ def test_aws_payload_tagging_s3_invalid_config(self): with pytest.raises(Exception): s3.list_objects(bucket="mybucket") + @pytest.mark.snapshot(ignores=snapshot_ignores) + @pytest.mark.skipif( + PYTHON_VERSION_INFO < (3, 8), + reason="Skipping for older py versions whose latest supported moto versions don't have the right dynamodb api", + ) + @mock_dynamodb + def test_dynamodb_payload_tagging(self): + with self.override_config( + "botocore", + dict(payload_tagging_request="all", payload_tagging_response="all", payload_tagging_services="s3,dynamodb"), + ): + ddb = self.session.create_client("dynamodb", region_name="us-west-2") + pin = Pin(service=self.TEST_SERVICE) + pin._tracer = self.tracer + pin.onto(ddb) + + with self.override_config("botocore", dict(instrument_internals=True)): + ddb.create_table( + TableName="foobar", + AttributeDefinitions=[{"AttributeName": "myattr", "AttributeType": "S"}], + KeySchema=[{"AttributeName": "myattr", "KeyType": "HASH"}], + BillingMode="PAY_PER_REQUEST", + ) + ddb.put_item(TableName="foobar", Item={"myattr": {"S": "baz"}}) + ddb.get_item(TableName="foobar", Key={"myattr": {"S": "baz"}}) + + spans = self.get_spans() + assert spans + span = spans[0] + assert len(spans) == 6 + assert_is_measured(span) + assert span.get_tag("aws.operation") == "CreateTable" + assert span.get_tag("component") == "botocore" + assert span.get_tag("span.kind"), "client" + assert_span_http_status_code(span, 200) + assert span.service == "test-botocore-tracing.dynamodb" + assert span.resource == "dynamodb.createtable" + + span = spans[1] + assert span.name == "botocore.parsers.parse" + assert span.get_tag("component") == "botocore" + assert span.get_tag("span.kind"), "client" + assert span.service == "test-botocore-tracing.dynamodb" + assert span.resource == "botocore.parsers.parse" + + span = spans[2] + assert span.get_tag("aws.operation") == "PutItem" + # Since the dynamodb_primary_key_names_for_tables isn't configured, we + # cannot create span pointers for this item. + assert not span._links + @pytest.mark.skip(reason="broken during period of skipping on main branch") @pytest.mark.snapshot(ignores=snapshot_ignores) @mock_s3 diff --git a/tests/snapshots/tests.contrib.botocore.test.BotocoreTest.test_dynamodb_payload_tagging.json b/tests/snapshots/tests.contrib.botocore.test.BotocoreTest.test_dynamodb_payload_tagging.json new file mode 100644 index 00000000000..69c87ff588e --- /dev/null +++ b/tests/snapshots/tests.contrib.botocore.test.BotocoreTest.test_dynamodb_payload_tagging.json @@ -0,0 +1,275 @@ +[[ + { + "name": "dynamodb.command", + "service": "test-botocore-tracing.dynamodb", + "resource": "dynamodb.getitem", + "trace_id": 0, + "span_id": 1, + "parent_id": 0, + "type": "http", + "error": 0, + "meta": { + "_dd.base_service": "tests.contrib.botocore", + "_dd.p.dm": "-0", + "_dd.p.tid": "67a6776f00000000", + "aws.agent": "botocore", + "aws.dynamodb.table_name": "foobar", + "aws.operation": "GetItem", + "aws.region": "us-west-2", + "aws.request.body.Key.myattr.S": "baz", + "aws.request.body.TableName": "foobar", + "aws.requestid": "1pGBOmkOZ1BdOWIHeWnkX96CO0n93XaZSqtfcgRZJhe6hrREgPCs", + "aws.response.body.HTTPHeaders.date": "Fri, 07 Feb 2025 21:13:20 GMT", + "aws.response.body.HTTPHeaders.server": "amazon.com", + "aws.response.body.HTTPHeaders.x-amz-crc32": "357125523", + "aws.response.body.HTTPHeaders.x-amzn-requestid": "1pGBOmkOZ1BdOWIHeWnkX96CO0n93XaZSqtfcgRZJhe6hrREgPCs", + "aws.response.body.HTTPStatusCode": "200", + "aws.response.body.RequestId": "1pGBOmkOZ1BdOWIHeWnkX96CO0n93XaZSqtfcgRZJhe6hrREgPCs", + "aws.response.body.RetryAttempts": "0", + "aws_service": "dynamodb", + "component": "botocore", + "http.status_code": "200", + "language": "python", + "region": "us-west-2", + "runtime-id": "ef4281e602a34aa3b78873db2aa0da91", + "span.kind": "client", + "tablename": "foobar" + }, + "metrics": { + "_dd.measured": 1, + "_dd.top_level": 1, + "_dd.tracer_kr": 1.0, + "_sampling_priority_v1": 1, + "process_id": 7842, + "retry_attempts": 0 + }, + "duration": 898096696, + "start": 1738962799557241364 + }, + { + "name": "botocore.parsers.parse", + "service": "test-botocore-tracing.dynamodb", + "resource": "botocore.parsers.parse", + "trace_id": 0, + "span_id": 2, + "parent_id": 1, + "type": "", + "error": 0, + "meta": { + "_dd.base_service": "tests.contrib.botocore", + "component": "botocore", + "span.kind": "client" + }, + "duration": 328440, + "start": 1738962800007520310 + }], +[ + { + "name": "dynamodb.command", + "service": "test-botocore-tracing.dynamodb", + "resource": "dynamodb.createtable", + "trace_id": 1, + "span_id": 1, + "parent_id": 0, + "type": "http", + "error": 0, + "meta": { + "_dd.base_service": "tests.contrib.botocore", + "_dd.p.dm": "-0", + "_dd.p.tid": "67a6776d00000000", + "aws.agent": "botocore", + "aws.dynamodb.table_name": "foobar", + "aws.operation": "CreateTable", + "aws.region": "us-west-2", + "aws.request.body.AttributeDefinitions.0.AttributeName": "myattr", + "aws.request.body.AttributeDefinitions.0.AttributeType": "S", + "aws.request.body.BillingMode": "PAY_PER_REQUEST", + "aws.request.body.KeySchema.0.AttributeName": "myattr", + "aws.request.body.KeySchema.0.KeyType": "HASH", + "aws.request.body.TableName": "foobar", + "aws.requestid": "s5qfbhhfkr4Go9yZWDL5eSDNflqO9AY7R1PtdbeHgZSnABZRXWwc", + "aws.response.body.HTTPHeaders.date": "Fri, 07 Feb 2025 21:13:17 GMT", + "aws.response.body.HTTPHeaders.server": "amazon.com", + "aws.response.body.HTTPHeaders.x-amz-crc32": "725802381", + "aws.response.body.HTTPHeaders.x-amzn-requestid": "s5qfbhhfkr4Go9yZWDL5eSDNflqO9AY7R1PtdbeHgZSnABZRXWwc", + "aws.response.body.HTTPStatusCode": "200", + "aws.response.body.RequestId": "s5qfbhhfkr4Go9yZWDL5eSDNflqO9AY7R1PtdbeHgZSnABZRXWwc", + "aws.response.body.RetryAttempts": "0", + "aws_service": "dynamodb", + "component": "botocore", + "http.status_code": "200", + "language": "python", + "region": "us-west-2", + "runtime-id": "ef4281e602a34aa3b78873db2aa0da91", + "span.kind": "client", + "tablename": "foobar" + }, + "metrics": { + "_dd.measured": 1, + "_dd.top_level": 1, + "_dd.tracer_kr": 1.0, + "_sampling_priority_v1": 1, + "process_id": 7842, + "retry_attempts": 0 + }, + "duration": 1169517214, + "start": 1738962797488686721 + }, + { + "name": "botocore.parsers.parse", + "service": "test-botocore-tracing.dynamodb", + "resource": "botocore.parsers.parse", + "trace_id": 1, + "span_id": 2, + "parent_id": 1, + "type": "", + "error": 0, + "meta": { + "_dd.base_service": "tests.contrib.botocore", + "component": "botocore", + "span.kind": "client" + }, + "duration": 680298, + "start": 1738962798196385705 + }], +[ + { + "name": "dynamodb.command", + "service": "test-botocore-tracing.dynamodb", + "resource": "dynamodb.putitem", + "trace_id": 2, + "span_id": 1, + "parent_id": 0, + "type": "http", + "error": 0, + "meta": { + "_dd.base_service": "tests.contrib.botocore", + "_dd.p.dm": "-0", + "_dd.p.tid": "67a6776e00000000", + "aws.agent": "botocore", + "aws.dynamodb.table_name": "foobar", + "aws.operation": "PutItem", + "aws.region": "us-west-2", + "aws.request.body.Item.myattr.S": "baz", + "aws.request.body.TableName": "foobar", + "aws.requestid": "l8KoXDDLWxK9lyqrrMRmtbdenA29koyXlCZEOlqgXgh46RkAzTiV", + "aws.response.body.HTTPHeaders.date": "Fri, 07 Feb 2025 21:13:19 GMT", + "aws.response.body.HTTPHeaders.server": "amazon.com", + "aws.response.body.HTTPHeaders.x-amz-crc32": "2745614147", + "aws.response.body.HTTPHeaders.x-amzn-requestid": "l8KoXDDLWxK9lyqrrMRmtbdenA29koyXlCZEOlqgXgh46RkAzTiV", + "aws.response.body.HTTPStatusCode": "200", + "aws.response.body.RequestId": "l8KoXDDLWxK9lyqrrMRmtbdenA29koyXlCZEOlqgXgh46RkAzTiV", + "aws.response.body.RetryAttempts": "0", + "aws_service": "dynamodb", + "component": "botocore", + "http.status_code": "200", + "language": "python", + "region": "us-west-2", + "runtime-id": "ef4281e602a34aa3b78873db2aa0da91", + "span.kind": "client", + "tablename": "foobar" + }, + "metrics": { + "_dd.measured": 1, + "_dd.top_level": 1, + "_dd.tracer_kr": 1.0, + "_sampling_priority_v1": 1, + "process_id": 7842, + "retry_attempts": 0 + }, + "duration": 897531031, + "start": 1738962798659199416 + }, + { + "name": "botocore.parsers.parse", + "service": "test-botocore-tracing.dynamodb", + "resource": "botocore.parsers.parse", + "trace_id": 2, + "span_id": 2, + "parent_id": 1, + "type": "", + "error": 0, + "meta": { + "_dd.base_service": "tests.contrib.botocore", + "component": "botocore", + "span.kind": "client" + }, + "duration": 188248, + "start": 1738962799115125322 + }], +[ + { + "name": "sqs.command", + "service": "aws.sqs", + "resource": "sqs.listqueues", + "trace_id": 3, + "span_id": 1, + "parent_id": 0, + "type": "http", + "error": 0, + "meta": { + "_dd.base_service": "tests.contrib.botocore", + "_dd.p.dm": "-0", + "_dd.p.tid": "67a6776d00000000", + "aws.agent": "botocore", + "aws.operation": "ListQueues", + "aws.region": "us-east-1", + "aws.requestid": "8BQ748A97HZTB3DQNICVX52K4H4KRCK9BWKBM3QX1FE0F9DJ4481", + "aws_service": "sqs", + "component": "botocore", + "http.status_code": "200", + "language": "python", + "region": "us-east-1", + "runtime-id": "ef4281e602a34aa3b78873db2aa0da91", + "span.kind": "client" + }, + "metrics": { + "_dd.measured": 1, + "_dd.top_level": 1, + "_dd.tracer_kr": 1.0, + "_sampling_priority_v1": 1, + "process_id": 7842, + "retry_attempts": 0 + }, + "duration": 17043485, + "start": 1738962797437241544 + }], +[ + { + "name": "sqs.command", + "service": "aws.sqs", + "resource": "sqs.createqueue", + "trace_id": 4, + "span_id": 1, + "parent_id": 0, + "type": "http", + "error": 0, + "meta": { + "_dd.base_service": "tests.contrib.botocore", + "_dd.p.dm": "-0", + "_dd.p.tid": "67a6776d00000000", + "aws.agent": "botocore", + "aws.operation": "CreateQueue", + "aws.region": "us-east-1", + "aws.requestid": "P5XR375SOHFTGCNG78FP0M00O7J8TR5Y01KWQXDNSAC2MR17BVSN", + "aws.sqs.queue_name": "Test", + "aws_service": "sqs", + "component": "botocore", + "http.status_code": "200", + "language": "python", + "queuename": "Test", + "region": "us-east-1", + "runtime-id": "ef4281e602a34aa3b78873db2aa0da91", + "span.kind": "client" + }, + "metrics": { + "_dd.measured": 1, + "_dd.top_level": 1, + "_dd.tracer_kr": 1.0, + "_sampling_priority_v1": 1, + "process_id": 7842, + "retry_attempts": 0 + }, + "duration": 9204293, + "start": 1738962797455089485 + }]] From 408d0644cf40602f1ce4335c0ca05519c4f3742f Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Thu, 20 Feb 2025 20:05:16 +0000 Subject: [PATCH 2/2] chore(pymongo): add additional asserts on resource names and mongodb.query span tag [backport 3.0] (#12448) Backport 957ebc45a5bcfbd583063edd5eb89822a6f37937 from #12333 to 3.0. In preparation of @ajgajg1134 's https://github.com/DataDog/dd-trace-py/pull/12311, this PR adds additional asserts for the current state of mongodb resource names and mongodb.query. Right now the tests show that there's a diff of `"` and `'` in the current behavior. ## Checklist - [x] PR author has checked that all the criteria below are met - The PR description includes an overview of the change - The PR description articulates the motivation for the change - The change includes tests OR the PR description describes a testing strategy - The PR description notes risks associated with the change, if any - Newly-added code is easy to change - The change follows the [library release note guidelines](https://ddtrace.readthedocs.io/en/stable/releasenotes.html) - The change includes or references documentation updates if necessary - Backport labels are set (if [applicable](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting)) ## Reviewer Checklist - [x] Reviewer has checked that all the criteria below are met - Title is accurate - All changes are related to the pull request's stated goal - Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes - Testing strategy adequately addresses listed risks - Newly-added code is easy to change - Release note makes sense to a user of the library - If necessary, author has acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment - Backport labels are set in a manner that is consistent with the [release branch maintenance policy](https://ddtrace.readthedocs.io/en/latest/contributing.html#backporting) Co-authored-by: wantsui --- tests/contrib/pymongo/test.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/contrib/pymongo/test.py b/tests/contrib/pymongo/test.py index 31bd45b4674..128d04802f5 100644 --- a/tests/contrib/pymongo/test.py +++ b/tests/contrib/pymongo/test.py @@ -292,8 +292,10 @@ def test_insert_find(self): # confirm query tag for find all assert spans[-2].get_tag("mongodb.query") is None + assert spans[-2].resource == "find teams" # confirm query tag find with query criteria on name + assert spans[-1].resource == 'find teams {"name": "?"}' assert spans[-1].get_tag("mongodb.query") == "{'name': '?'}" def test_update_ot(self): @@ -402,6 +404,12 @@ def test_rowcount(self): one_row_span = spans[2] two_row_span = spans[3] + # Assert resource names and mongodb.query + assert one_row_span.resource == 'find songs {"name": "?"}' + assert one_row_span.get_tag("mongodb.query") == "{'name': '?'}" + assert two_row_span.resource == 'find songs {"artist": "?"}' + assert two_row_span.get_tag("mongodb.query") == "{'artist': '?'}" + assert one_row_span.name == "pymongo.cmd" assert one_row_span.get_metric("db.row_count") == 1 assert two_row_span.get_metric("db.row_count") == 2 @@ -426,6 +434,8 @@ def test_patch_pymongo_client_after_import(self): spans = tracer.pop() assert len(spans) == 2 assert spans[1].name == "pymongo.cmd" + assert spans[1].resource == "buildinfo 1" + assert spans[1].get_tag("mongodb.query") is None class TestPymongoPatchDefault(TracerTestCase, PymongoCore):