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

New implementation of firehose_exporter #414

Closed

Conversation

ArthurHlt
Copy link
Contributor

This PR is associated to PR cloudfoundry/firehose_exporter#63 on firehose_exporter.
Please see this pull request for more context.

Here, it will only change dashboards consequently to the change of http_start_stop, set correct ctl for new firehose exporter and fix previous operator for firehose.

Dashboard latency include new panels which look like this a the end:

Capture d’écran 2021-03-29 à 11 37 38

@benjaminguttmann-avtq
Copy link
Contributor

Hi @ArthurHlt had a look at this one and there are a couple things I want to mention,

as I read in the firehose_exporter it is fully "retro" compatible but we modify the dashboards to only work with the new metrics, maybe we should introduce a version 2 of the dashboards handling the new metrics. In this case it might be worth checking if we can combine it with the improvements in #423 by @thehandsomezebra .

So we can get both in there.

I think we have some alerts relying on the firehose_http_start_stop_requests, do we need to adjust them as well?

I think this part of the README needs to get adjusted as well, right? As we need some additional certificate now?

In addition, I am wondering if we should add some instructions, ops files to help people that still rely on the 'retro' compatible version.

@thehandsomezebra
Copy link
Contributor

thehandsomezebra commented Oct 8, 2021

Thanks @benjaminguttmann-avtq for pinging me on this one.
I checked this out, and I think that @ArthurHlt 's PR would need just a couple adjustments to not conflict with my PR #423 AND still be backwards compatible..

Here's my thoughts:

I do have concerns about the other files like jobs/firehose_exporter/spec -- If you are removing properties in the spec file, I'm curious on how this would be backwards compatible? At what point would the firehose_exporter PR be classified as breaking changes..?

@psycofdj
Copy link
Contributor

closing in favor of #461

@psycofdj psycofdj closed this Apr 25, 2023
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.

4 participants