dd-trace-py icon indicating copy to clipboard operation
dd-trace-py copied to clipboard

chore(span_sampling): add jsonschema for span sampling

Open ZStriker19 opened this issue 1 year ago • 3 comments

Description

It was proposed by JD that I should add a schema rather than do a lot of validation for the span sampling json we pull via envar or file.

Checklist

Reviewer Checklist

  • [ ] Title is accurate.
  • [ ] Description motivates each change.
  • [ ] No unnecessary changes were introduced in this PR.
  • [ ] PR cannot be broken up into smaller PRs.
  • [ ] Avoid breaking API changes unless absolutely necessary.
  • [ ] Tests provided or description of manual testing performed is included in the code or PR.
  • [ ] Release note has been added for fixes and features, or else changelog/no-changelog label added.
  • [ ] All relevant GitHub issues are correctly linked.
  • [ ] Backports are identified and tagged with Mergifyio.
  • [ ] Add to milestone.

ZStriker19 avatar Aug 09 '22 01:08 ZStriker19

@ZStriker19 this pull request is now in conflict 😩

mergify[bot] avatar Aug 10 '22 19:08 mergify[bot]

With the addition of the new package this test is failing: https://app.circleci.com/pipelines/github/DataDog/dd-trace-py/20247/workflows/9b33a31b-e2e2-4c9b-9e27-ddeec7800120/jobs/1371051. We'll need to update this case to handle/reformat pkgutil_resolve_name

Something like this could work. Or we could just replace all - with _ in pkg_resources_ws and importlib_pkgs.

def test_get_distributions():
    """use pkg_resources to validate package names and versions returned by get_distributions()"""
    import pkg_resources

    pkg_resources_ws = {pkg.project_name for pkg in pkg_resources.working_set}

    importlib_pkgs = set()
    for pkg in get_distributions():
        assert pkg.name
        assert pkg.version
        assert os.path.exists(pkg.path)
        # The package name in typing_extensions-4.x.x.dist-info/METADATA is set to `typing_extensions`
        # this is inconsistent with the package name found in pkg_resources. The block below corrects this.
        # The correct package name is typing-extensions.
    +  # The issue exists in pkgutil-resolve-name package.
        if pkg.name == "typing_extensions" and "typing-extensions" in pkg_resources_ws:
            importlib_pkgs.add("typing-extensions")
    +  elif pkg.name == "pkgutil_resolve_name" and "pkgutil-resolve-name" in pkg_resources_ws:
    +      importlib_pkgs.add("pkgutil-resolve-name")
        else:
            importlib_pkgs.add(pkg.name)

    # assert that pkg_resources and importlib.metadata return the same packages
    assert pkg_resources_ws == importlib_pkgs

mabdinur avatar Aug 10 '22 21:08 mabdinur

@ZStriker19 this pull request is now in conflict 😩

mergify[bot] avatar Aug 11 '22 11:08 mergify[bot]