Skip to content

enable test for process tags in profiles in dotnet and go#5953

Merged
vandonr merged 8 commits intomainfrom
vandonr/sys
Jan 22, 2026
Merged

enable test for process tags in profiles in dotnet and go#5953
vandonr merged 8 commits intomainfrom
vandonr/sys

Conversation

@vandonr
Copy link
Copy Markdown
Contributor

@vandonr vandonr commented Jan 7, 2026

Motivation

test DataDog/dd-trace-dotnet#7715 (need to wait for release before this can be merged)
plus go implem that was done a while ago

Changes

Also added a check that we received some data because the test was passing if profiler didn't run at all

Workflow

  1. ⚠️ Create your PR as draft ⚠️
  2. Work on you PR until the CI passes
  3. Mark it as ready for review
    • Test logic is modified? -> Get a review from RFC owner.
    • Framework is modified, or non obvious usage of it -> get a review from R&P team

🚀 Once your PR is reviewed and the CI green, you can merge it!

🛟 #apm-shared-testing 🛟

Reviewer checklist

  • Anything but tests/ or manifests/ is modified ? I have the approval from R&P team
  • A docker base image is modified?
    • the relevant build-XXX-image label is present
  • A scenario is added, removed or renamed?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Jan 7, 2026

CODEOWNERS have been resolved as:

manifests/cpp_nginx.yml                                                 @DataDog/system-tests-core
manifests/dotnet.yml                                                    @DataDog/apm-dotnet @DataDog/asm-dotnet
manifests/nodejs.yml                                                    @DataDog/dd-trace-js
tests/test_profiling.py                                                 @DataDog/system-tests-core

@vandonr vandonr marked this pull request as ready for review January 12, 2026 15:17
@vandonr vandonr requested a review from a team as a code owner January 12, 2026 15:17
Comment thread tests/test_profiling.py Outdated
@features.process_tags
@missing_feature(
condition=context.library.name not in ("java", "python"),
condition=context.library.name not in ("java", "python", "dotnet", "golang"),
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please consider moving those decorators to the manifest files

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done, not sure about the cpp and rust manifest files, they are pretty empty for now

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thank you, rust and cpp are only using parametric tests that's why their manifests are shorter. It means that you probably don't need to add an entry

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ah ok, yeah makes sense, I'll undo the change for those

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

php and ruby already excluded the whole file.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the cpp tests are failing :o I've been lied to

@vandonr vandonr requested review from a team as code owners January 20, 2026 10:50
@vandonr vandonr requested review from Anilm3 and removed request for a team January 20, 2026 10:50
Comment thread manifests/cpp.yml Outdated
@cbeauchesne cbeauchesne dismissed their stale review January 22, 2026 10:27

changes has been applied

@vandonr vandonr merged commit 7a988a1 into main Jan 22, 2026
765 of 772 checks passed
@vandonr vandonr deleted the vandonr/sys branch January 22, 2026 13:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants