chore: [SVLS-5262] Span Pointer standard hashing #10687

apiarian-datadog · 2024-09-17T16:03:22Z

Adding a standard hashing function for Span Pointers. This continues to be a private API for now.

Checklist

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
The change includes or references documentation updates if necessary
Backport labels are set (if applicable)

Reviewer Checklist

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

apiarian-datadog · 2024-09-17T16:03:42Z

built on top of #10685

datadog-dd-trace-py-rkomorn · 2024-09-17T16:17:46Z

Datadog Report

Branch report: aleksandr.pasechnik/svls-5262-3-standard-hashing-function
Commit report: bd01c43
Test service: dd-trace-py

✅ 0 Failed, 962 Passed, 324 Skipped, 28m 30.31s Total duration (9m 12.53s time saved)

github-actions · 2024-09-17T16:22:08Z

CODEOWNERS have been resolved as:

tests/tracer/test_span_pointers.py                                      @DataDog/apm-sdk-api-python
ddtrace/_trace/_span_pointer.py                                         @DataDog/apm-sdk-api-python

pr-commenter · 2024-09-17T16:37:36Z

Benchmarks

Benchmark execution time: 2024-09-19 19:04:48

Comparing candidate commit bd01c43 in PR branch aleksandr.pasechnik/svls-5262-3-standard-hashing-function with baseline commit b07f73a in branch main.

Found 1 performance improvements and 0 performance regressions! Performance is the same for 352 metrics, 47 unstable metrics.

scenario:iast_aspects-aspect_iast_do_zfill

🟩 max_rss_usage [-2.471MB; -2.141MB] or [-8.321%; -7.210%]

ddtrace/_trace/_span_pointer.py

…th detailed tests

…wing an exception

wip wip Update scripts/freshvenvs.py Co-authored-by: datadog-datadog-prod-us1[bot] <88084959+datadog-datadog-prod-us1[bot]@users.noreply.github.com> wip wip wip wip wip wip wip wip wip move to scripts move to scripts wip wip wip wip wip wip wip wip wip wip wip chore: [SVLS-5262] Span Pointer standard hashing (#10687) Adding a standard hashing function for Span Pointers. This continues to be a private API for now. - [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)) - [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) fix(llmobs): safely handle non-json serializable arguments (#10694) Safely handle non-JSON serializable tag arguments in LLMObs.annotate() and the OpenAI/LangChain/Bedrock/Anthropic integrations. - LLMObs.annotate(): we previously just discarded the entire argument to annotate if the argument was non-JSON serializable. Now, we safely convert non-JSON serializable fields/objects to a default placeholder text, meaning users can still send *some* data even if some of it may be invalid. - Same idea with each integration, we ensure we safely handle non-JSON serializable args and default to placeholder texts if necessary. - We've moved all json.dumps() call into a private helper `safe_json()` which does the above for us. Note: This PR removes some tests in `test_llmobs_service.py` regarding truly unserializable objects as this is highly unlikely, someone would have to go out of their way to make a truly unserializable object (i.e. override `__repr__()` with a non-json serializable value). We still catch these so any resulting crashes should not be from our code. - [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)) - [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) fix(llmobs): ensure integration metrics are disabled in agentless mode (#10712) Follows up on #10084, which missed a condition where agentless mode should always disable integration metrics. Adds standard testing cases for the `BaseLLMIntegration` class, which was missing prior to this (and thus the reason for missing this edge case prior) - [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)) - [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) chore(crashtracking): add python version to crashtracker tags (#10720) During escalations it's currently difficult to find the exact Python version relevant for a defect. This is a simple fix for that. I'm recommending a backport because 1. The fix is simple 2. 2.11 and onward introduce crashtracker, and we're still dealing with crashes from those versions - [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)) - [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) minor changes minor changes

Adding a standard hashing function for Span Pointers. This continues to be a private API for now. ## 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)

apiarian-datadog mentioned this pull request Sep 17, 2024

feat(tracer): [SVLS-5262] S3.PutObject Span Pointers #10639

Closed

2 tasks

apiarian-datadog added the changelog/no-changelog A changelog entry is not required for this PR. label Sep 17, 2024

apiarian-datadog mentioned this pull request Sep 17, 2024

chore: [SVLS-5262] AWS S3 PutObject pointer calc #10689

Merged

2 tasks

apiarian-datadog force-pushed the aleksandr.pasechnik/svls-5262-2-span-pointer-descriptions branch from 21ca586 to 5ace397 Compare September 17, 2024 18:04

apiarian-datadog force-pushed the aleksandr.pasechnik/svls-5262-3-standard-hashing-function branch from c04dff7 to 30ed9a8 Compare September 17, 2024 18:06

apiarian-datadog marked this pull request as ready for review September 17, 2024 19:03

apiarian-datadog requested a review from a team as a code owner September 17, 2024 19:03

apiarian-datadog requested review from rachelyangdog and removed request for a team September 17, 2024 19:03

ZStriker19 reviewed Sep 18, 2024

View reviewed changes

ddtrace/_trace/_span_pointer.py Outdated Show resolved Hide resolved

Base automatically changed from aleksandr.pasechnik/svls-5262-2-span-pointer-descriptions to aleksandr.pasechnik/svls-5262-1-extract-span-pointers September 18, 2024 17:42

ZStriker19 approved these changes Sep 18, 2024

View reviewed changes

apiarian-datadog force-pushed the aleksandr.pasechnik/svls-5262-1-extract-span-pointers branch from 1d95cca to bd84ac8 Compare September 19, 2024 13:10

apiarian-datadog force-pushed the aleksandr.pasechnik/svls-5262-3-standard-hashing-function branch from 30ed9a8 to 1080854 Compare September 19, 2024 13:47

apiarian-datadog changed the base branch from aleksandr.pasechnik/svls-5262-1-extract-span-pointers to aleksandr.pasechnik/svls-5262-2-span-pointer-descriptions September 19, 2024 13:48

ZStriker19 reviewed Sep 19, 2024

View reviewed changes

ddtrace/_trace/_span_pointer.py Outdated Show resolved Hide resolved

apiarian-datadog force-pushed the aleksandr.pasechnik/svls-5262-2-span-pointer-descriptions branch from a4f26eb to b82a7bd Compare September 19, 2024 15:02

apiarian-datadog force-pushed the aleksandr.pasechnik/svls-5262-3-standard-hashing-function branch from 1080854 to 9d8d7b0 Compare September 19, 2024 15:03

Base automatically changed from aleksandr.pasechnik/svls-5262-2-span-pointer-descriptions to main September 19, 2024 17:42

An error occurred while trying to automatically change base from aleksandr.pasechnik/svls-5262-2-span-pointer-descriptions to main September 19, 2024 17:42

apiarian-datadog added 3 commits September 19, 2024 13:43

[SVLS-5262] Span Pointer standard hashing

b3680ab

[SVLS-5262] change _standard_hashing_function to a normal function wi…

12bcb72

…th detailed tests

[SVLS-5262] log warnings in standard hashing function instead of thro…

8c78bcd

…wing an exception

apiarian-datadog force-pushed the aleksandr.pasechnik/svls-5262-3-standard-hashing-function branch from 9d8d7b0 to 8c78bcd Compare September 19, 2024 18:22

ZStriker19 approved these changes Sep 19, 2024

View reviewed changes

apiarian-datadog changed the title ~~feat(tracer): [SVLS-5262] Span Pointer standard hashing~~ chore: [SVLS-5262] Span Pointer standard hashing Sep 19, 2024

[SVLS-5262] this random choice does not need to be secure

bd01c43

apiarian-datadog merged commit 6190605 into main Sep 19, 2024
573 checks passed

apiarian-datadog deleted the aleksandr.pasechnik/svls-5262-3-standard-hashing-function branch September 19, 2024 19:48

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore: [SVLS-5262] Span Pointer standard hashing #10687

chore: [SVLS-5262] Span Pointer standard hashing #10687

apiarian-datadog commented Sep 17, 2024 •

edited by ZStriker19

Loading

apiarian-datadog commented Sep 17, 2024

datadog-dd-trace-py-rkomorn bot commented Sep 17, 2024 •

edited

Loading

github-actions bot commented Sep 17, 2024

pr-commenter bot commented Sep 17, 2024 •

edited

Loading

chore: [SVLS-5262] Span Pointer standard hashing #10687

chore: [SVLS-5262] Span Pointer standard hashing #10687

Conversation

apiarian-datadog commented Sep 17, 2024 • edited by ZStriker19 Loading

Checklist

Reviewer Checklist

apiarian-datadog commented Sep 17, 2024

datadog-dd-trace-py-rkomorn bot commented Sep 17, 2024 • edited Loading

Datadog Report

github-actions bot commented Sep 17, 2024

pr-commenter bot commented Sep 17, 2024 • edited Loading

Benchmarks

scenario:iast_aspects-aspect_iast_do_zfill

apiarian-datadog commented Sep 17, 2024 •

edited by ZStriker19

Loading

datadog-dd-trace-py-rkomorn bot commented Sep 17, 2024 •

edited

Loading

pr-commenter bot commented Sep 17, 2024 •

edited

Loading