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

feature: count json objects with same value #149

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

parsa97
Copy link

@parsa97 parsa97 commented Apr 28, 2022

if we want to count json objects with same values we can use
countbylabel type in the metric configuration

@parsa97 parsa97 force-pushed the add_count_for_json_values branch 2 times, most recently from 30d4492 to 39a9e37 Compare April 28, 2022 13:51
Copy link
Contributor

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

I like the idea, but I think we need to refactor some things first.

exporter/collector.go Outdated Show resolved Hide resolved
Copy link
Contributor

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

Ok, I merged #152 as a refactor to help with this PR. Please rebase with those changes.

@SuperQ
Copy link
Contributor

SuperQ commented Jun 6, 2023

This needs to be rebased.

@parsa97 parsa97 force-pushed the add_count_for_json_values branch 2 times, most recently from 5c2d471 to de7c4c7 Compare June 6, 2023 17:34
@parsa97
Copy link
Author

parsa97 commented Jun 6, 2023

Hey Ben, PTAL

Copy link
Contributor

@SuperQ SuperQ left a comment

Choose a reason for hiding this comment

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

Thanks

@SuperQ SuperQ requested a review from rustycl0ck June 6, 2023 19:36
@vandud
Copy link

vandud commented Sep 6, 2023

@SuperQ @rustycl0ck please, take a look at this issue again

labels:
environment: beta # static label
state: '{}' # dynamic label
mincount: 1
Copy link
Contributor

@SuperQ SuperQ Sep 6, 2023

Choose a reason for hiding this comment

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

This mincount is not exactly obvious without reading the code. Let's document it in the example config. Also, for the config, maybe a nicer name.

Suggested change
mincount: 1
# The minimum count of labels required to expose the metric for each label. Defaults to 1.
minimumCount: 1

Copy link
Author

Choose a reason for hiding this comment

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

@SuperQ @rustycl0ck please, take a look at this issue again

@parsa97 parsa97 force-pushed the add_count_for_json_values branch from de7c4c7 to 5632095 Compare September 6, 2023 18:05
@parsa97 parsa97 requested a review from SuperQ September 7, 2023 07:56
@parsa97 parsa97 force-pushed the add_count_for_json_values branch from 5632095 to 1b9f9f7 Compare September 11, 2023 06:45
if we want to count json objects with same values we can use
countbylabel type in the metric configuration

Signed-off-by: Parsa <[email protected]>
@parsa97 parsa97 force-pushed the add_count_for_json_values branch 2 times, most recently from 447d93d to f4d2d10 Compare September 11, 2023 13:38
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.

3 participants