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

feat(query): namespace cardinality publisher #1294

Merged
merged 18 commits into from
Dec 7, 2021

Conversation

alextheimer
Copy link
Contributor

@alextheimer alextheimer commented Nov 30, 2021

Pull Request checklist

  • The commit(s) message(s) follows the contribution guidelines ?
  • Tests for the changes have been added (for bug fixes / features) ?
  • Docs have been added / updated (for bug fixes / features) ?

New behavior :
Adds a TenantIngestionMetering instance to the leader NodeCoordinatorActor (via the ShardManager). This periodically queries a NodeCoordinatorActor for namespace-level cardinality data, then it publishes the results as Kamon metrics.

Copy link
Member

@vishramachandran vishramachandran left a comment

Choose a reason for hiding this comment

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

We should wait until the other PR goes through.
Comments added for your consideration.

@alextheimer alextheimer force-pushed the card-ts branch 2 times, most recently from 1dce982 to 1dd863b Compare December 2, 2021 20:58
@alextheimer alextheimer force-pushed the card-ts branch 2 times, most recently from 8190b92 to 731d2ec Compare December 2, 2021 21:32
@alextheimer
Copy link
Contributor Author

Rebased with the most recent develop updates, and confirmed to work locally.

@alextheimer
Copy link
Contributor Author

alextheimer commented Dec 4, 2021

2c73554 smuggles in a test that should have been included in the last PR

Copy link
Member

@vishramachandran vishramachandran left a comment

Choose a reason for hiding this comment

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

👍 Please address the two minor suggestions before pushing

@alextheimer alextheimer merged commit 95b91c4 into filodb:develop Dec 7, 2021
@alextheimer alextheimer deleted the card-ts branch December 7, 2021 04:43
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.

2 participants