Skip to content

parametric: enable Go for partial flushing and add additional test#1329

Merged
katiehockman merged 17 commits intomainfrom
katie.hockman/partialFLUSH
Aug 8, 2023
Merged

parametric: enable Go for partial flushing and add additional test#1329
katiehockman merged 17 commits intomainfrom
katie.hockman/partialFLUSH

Conversation

@katiehockman
Copy link
Copy Markdown
Contributor

@katiehockman katiehockman commented Jun 30, 2023

Description

Enables golang for partial flushing tests.
Adds additional partial flushing test to verify the behavior of DD_TRACE_PARTIAL_FLUSH_ENABLED.

There is a new test which ensures that partial flushing doesn't trigger if DD_TRACE_PARTIAL_FLUSH_ENABLED = "false" which passes for Go and Python, but doesn't actually disable the feature for .NET. This has been marked as a bug in the test file.

Motivation

Partial flushing is now supported in dd-trace-go on main. The additional tests should ensure that there is language consistency with the usage of DD_TRACE_PARTIAL_FLUSH_ENABLED.

Reviewer checklist

  • If this PR modifies anything else than strictly the default scenario, then add the run-all-scenarios label (more info).
  • CI is green
    • If not, failing jobs are not related to this change (and you are 100% sure about this statement)

Workflow

  1. ⚠️⚠️ Create your PR as draft
  2. Follow the style guidelines of this project (See how to easily lint the code)
  3. Work on you PR until the CI passes (if something not related to your task is failing, you can ignore it)
  4. Mark it as ready for review

Once your PR is reviewed, you can merge it! ❤️

@katiehockman katiehockman force-pushed the katie.hockman/partialFLUSH branch from 2b1b099 to f987db4 Compare August 1, 2023 19:27
@katiehockman katiehockman force-pushed the katie.hockman/partialFLUSH branch from c0742f5 to 3969c65 Compare August 2, 2023 16:40
@katiehockman katiehockman marked this pull request as ready for review August 3, 2023 16:10
@katiehockman katiehockman requested review from a team as code owners August 3, 2023 16:10
@katiehockman katiehockman changed the title WIP: parametric: enable Go for partial flushing and add additional test parametric: enable Go for partial flushing and add additional test Aug 3, 2023
Comment thread tests/parametric/README.md Outdated
Comment thread tests/parametric/test_partial_flushing.py
Comment thread utils/build/docker/golang/parametric/go.mod
Copy link
Copy Markdown
Member

@Kyle-Verhoog Kyle-Verhoog left a comment

Choose a reason for hiding this comment

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

(non-blocking-nit) can we add the @released decorator for the tests? This was recently added in #1442.

Example:

@scenarios.parametric
@released(python="0.36.0")
class Test_TracerUniversalServiceTagging:

…ts to fail. Inadvertently this bug allowed the 'default=ON' test to pass, so that is now being skipped since the default is actually OFF
@zacharycmontoya zacharycmontoya requested a review from a team as a code owner August 4, 2023 17:21
@katiehockman katiehockman merged commit be82953 into main Aug 8, 2023
@katiehockman katiehockman deleted the katie.hockman/partialFLUSH branch August 8, 2023 13:55
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.

7 participants