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

API: split pr for metrics protos #2246

Merged
merged 10 commits into from Jul 5, 2022
Merged

API: split pr for metrics protos #2246

merged 10 commits into from Jul 5, 2022

Conversation

smonero
Copy link
Contributor

@smonero smonero commented May 19, 2022

Description

It was requested to split this PR https://github.com/lyft/clutch/pull/2223/files to just the protos, so here they are :)

The idea here is that there will be a "metrics" service in Clutch that can get metrics from a store, such as m3 or something else. It follows a similar pattern as the diagnostics api in private. It could potentially "fan-out" to providers, as shown in the config proto. Something discussed at a meeting was that even though not necessarily all providers will have the same fields as prometheus does, it is ok for some of the fields to be blank.

Testing Performed

none needed

GitHub Issue

Fixes #

TODOs

@smonero smonero requested a review from a team as a code owner May 19, 2022 03:50
@smonero smonero mentioned this pull request May 27, 2022
1 task
@github-actions
Copy link

This PR has been marked as stale after 7 or more days of inactivity. Please have a maintainer add the on hold label if this PR should remain open. If there is no further activity or the on hold label is not added, this PR will be closed in 3 days.

@github-actions github-actions bot added the stale Issue hasn't had activity in awhile label May 31, 2022
@smonero smonero removed the stale Issue hasn't had activity in awhile label May 31, 2022
@smonero
Copy link
Contributor Author

smonero commented Jun 6, 2022

return type should be one:
provider style configuration
fanout to different providers?

enable in config section

example:
https://github.com/lyft/clutch-private/blob/main/backend/config/dash-config.yaml#L4-L9

topology uses data blob that it ingests

@github-actions
Copy link

This PR has been marked as stale after 7 or more days of inactivity. Please have a maintainer add the on hold label if this PR should remain open. If there is no further activity or the on hold label is not added, this PR will be closed in 3 days.

@github-actions github-actions bot added the stale Issue hasn't had activity in awhile label Jun 13, 2022
@smonero smonero removed the stale Issue hasn't had activity in awhile label Jun 15, 2022
@github-actions
Copy link

This PR has been marked as stale after 7 or more days of inactivity. Please have a maintainer add the on hold label if this PR should remain open. If there is no further activity or the on hold label is not added, this PR will be closed in 3 days.

@github-actions github-actions bot added the stale Issue hasn't had activity in awhile label Jun 22, 2022
@smonero smonero removed the stale Issue hasn't had activity in awhile label Jun 23, 2022
@github-actions
Copy link

This PR has been marked as stale after 7 or more days of inactivity. Please have a maintainer add the on hold label if this PR should remain open. If there is no further activity or the on hold label is not added, this PR will be closed in 3 days.

@github-actions github-actions bot added the stale Issue hasn't had activity in awhile label Jun 30, 2022
@smonero smonero removed the stale Issue hasn't had activity in awhile label Jul 3, 2022
Copy link
Collaborator

@danielhochman danielhochman left a comment

Choose a reason for hiding this comment

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

small nits

api/config/service/metrics/v1/metrics.proto Outdated Show resolved Hide resolved
api/metrics/v1/metrics.proto Outdated Show resolved Hide resolved
api/metrics/v1/metrics.proto Outdated Show resolved Hide resolved
@smonero smonero merged commit be66249 into main Jul 5, 2022
@smonero smonero deleted the metricsProtos branch July 5, 2022 23:03
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.

None yet

3 participants