Skip to content

allows list of weblogs in weblog declaration keys#5919

Merged
christophe-papazian merged 11 commits intomainfrom
christophe-papazian/allow_list_in_weblog_declaration_keys
Dec 30, 2025
Merged

allows list of weblogs in weblog declaration keys#5919
christophe-papazian merged 11 commits intomainfrom
christophe-papazian/allow_list_in_weblog_declaration_keys

Conversation

@christophe-papazian
Copy link
Copy Markdown
Contributor

@christophe-papazian christophe-papazian commented Dec 29, 2025

Motivation

weblog groups are cool and useful for python tracer to have a more readable manifest.

Changes

Allow weblog groups in manifests:

  • modify the parser to allow them in weblog declarations (both as keys and values)
  • modify the python manifest accordingly to use the new feature
  • improve unit tests for parser to check the new syntax is properly working

weblog group aliases should be usable whenever a normal weblog name can be used in the manifest.

Problem

To avoid problematic parsing and difficult json validation, only strings are manipulated at yaml level. The groups are then handled at the python level when parsing.

APPSEC-60404

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 Dec 29, 2025

CODEOWNERS have been resolved as:

manifests/python.yml                                                    @DataDog/apm-python @DataDog/asm-python
tests/test_the_test/manifests/manifests_parser_test/python.yml          @DataDog/system-tests-core
tests/test_the_test/test_manifest.py                                    @DataDog/system-tests-core
utils/manifest/_internal/parser.py                                      @DataDog/system-tests-core

@christophe-papazian christophe-papazian marked this pull request as ready for review December 30, 2025 12:42
@christophe-papazian christophe-papazian requested review from a team as code owners December 30, 2025 12:42
@christophe-papazian christophe-papazian requested review from ZStriker19, cbeauchesne and wconti27 and removed request for a team December 30, 2025 12:42
Copy link
Copy Markdown
Collaborator

@nccatoni nccatoni left a comment

Choose a reason for hiding this comment

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

You should use yaml aliases like the refs section. Please consider adding tests in the test the tests scenario

Comment thread utils/manifest/schema.json Outdated
Comment thread utils/manifest/_internal/parser.py Outdated
@christophe-papazian christophe-papazian enabled auto-merge (squash) December 30, 2025 16:18
new_entries: list[Condition] = []
all_weblogs: list[str] = []
for weblog in e[n]:
if not isinstance(weblog, str):
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.

Is there any valid reason for weblog not being a str ? If no, could you raise a TypeError?

@christophe-papazian christophe-papazian merged commit 380748e into main Dec 30, 2025
2097 checks passed
@christophe-papazian christophe-papazian deleted the christophe-papazian/allow_list_in_weblog_declaration_keys branch December 30, 2025 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants