Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Redis enterprise #2458

Merged
merged 146 commits into from
Aug 28, 2024
Merged

Conversation

j8-redis
Copy link
Contributor

@j8-redis j8-redis commented Aug 2, 2024

What does this PR do?

Adds metrics groups to the test for validation

Motivation

Metrics are unavailable and so additional testing was performed

Review checklist

  • PR has a meaningful title or PR has the no-changelog label attached
  • Feature or bugfix has tests
  • Git history is clean
  • If PR impacts documentation, docs team has been notified or an issue has been opened on the documentation repo
  • If this PR includes a log pipeline, please add a description describing the remappers and processors.

Additional Notes

Metrics that are only available in certain environments/installations can not be asserted; they are considered ephemeral.

Copy link
Contributor

@Kyle-Neale Kyle-Neale left a comment

Choose a reason for hiding this comment

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

These metrics should be added to the metrics.py and we should include as much of the metric output/examples in tests/data to be used as fixtures to server to our tests

Copy link
Contributor

@Kyle-Neale Kyle-Neale left a comment

Choose a reason for hiding this comment

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

Nit: Ideally we make changelog added additions minor fixes ( 1.1.0) but this is non-blocking. Approving 👍

@mgarabed mgarabed merged commit c255ce8 into DataDog:master Aug 28, 2024
28 checks passed
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