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

select metrics #12

Merged
merged 2 commits into from Apr 18, 2019
Merged

select metrics #12

merged 2 commits into from Apr 18, 2019

Conversation

bin3377
Copy link

@bin3377 bin3377 commented Apr 16, 2019

Copy link
Contributor

@samjsong samjsong left a comment

Choose a reason for hiding this comment

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

LGTM one pending comment

</match>
<filter prometheus.datapoint>
@type carbon_v2
</filter>
</filter>
# <filter prometheus.datapoint>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should remove the comment

Copy link
Author

Choose a reason for hiding this comment

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

It's for easier debugging. I'm ok to remove it

Copy link
Contributor

@lei-sumo lei-sumo left a comment

Choose a reason for hiding this comment

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

lgtm, left one question.

@@ -1,4 +1,58 @@
prometheus:
prometheusSpec:
remoteWrite:
- url: http://fluentd:9888/prometheus.metrics
- url: http://fluentd:9888/prometheus.metrics.state.daemonset
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these new url matching to the source endpoints created by Sam's PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think these urls need to match with the source endpoints created by my PR. Since fluentd accepts all of these metrics from prometheus and more or less treats them all the same (since they are all retagged with prometheus.datapoint). Then we separate the metrics again by job to send to the source endpoints on Sumo's end.

Copy link
Author

Choose a reason for hiding this comment

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

yes. they are transformed to same tag in datapoint filter stage so no impact. spliting them into different urls maybe benefit for the statistics later.

@bin3377 bin3377 merged commit bee421f into master Apr 18, 2019
@bin3377 bin3377 deleted the byi-filter-metrics branch April 18, 2019 17:58
psaia pushed a commit to psaia/sumologic-kubernetes-collection that referenced this pull request May 25, 2021
* adr 7

* Update and rename 0007-controlling-role-access-through-ad.md to 0008-controlling-role-access-through-ad.md
psaia pushed a commit to psaia/sumologic-kubernetes-collection that referenced this pull request May 25, 2021
* Instructions for using the adutil

* Update security team's filter (SumoLogic#99)

* PUBP-8182: provision elections collector (SumoLogic#98)

* PUBP-8182: provision elections collector

* PUBP-8182: provision elections collector

* PUBP-8182: renaming module to match repo name

* PUBP-8182: renaming output

Co-authored-by: Malcolm Jones <17786500+mlclmj@users.noreply.github.com>

* Add logging for AD parameters (SumoLogic#95)

* Log the vars subitted

* fix wording, enumerate commands directly

* drone sign

Co-authored-by: Marcin Suterski <msuterski@users.noreply.github.com>

* added collector for nytm/dv-drone in TF (SumoLogic#100)

* Adding collector for spg-inyt-subscription-api (SumoLogic#101)

* remove 'manageIndexes' from samizdat role (SumoLogic#102)

* Add ADR regarding AD groups and nesting (SumoLogic#12)

* adr 7

* Update and rename 0007-controlling-role-access-through-ad.md to 0008-controlling-role-access-through-ad.md

* Feature/add collector int fallback role (SumoLogic#104)

* adding new collector to the role

* adding new collector to the role

* Adutil instructions moved to dv-docs

* update link

Co-authored-by: Marcin Suterski <msuterski@users.noreply.github.com>
Co-authored-by: Shreelekha Tanna <shree.tanna@nytimes.com>
Co-authored-by: Malcolm Jones <17786500+mlclmj@users.noreply.github.com>
Co-authored-by: Prune Sebastien THOMAS <prune@lecentre.net>
Co-authored-by: satyabodharao <47533173+satyabodharao@users.noreply.github.com>
Co-authored-by: Shawn Bower <shawn.bower@gmail.com>
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