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

Add prometheus remote write support #12 and send logs events to loki #13

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

Conversation

dubyte
Copy link

@dubyte dubyte commented Mar 24, 2022

No description provided.

@ober37
Copy link

ober37 commented Mar 24, 2022

@dubyte thanks for this. There is some general confusion for me that's initially not obvious from this PR (I need to carve out time this week to study a bit more)...

  • The decision tree usage and it's architecturally purpose is not obvious to me, can you elaborate

  • I would like to have an ability to disable Loki integration and continue for logs / events to be via stdout, is this a new option required or something that is implied through the absence of an existing option?

  • Ultimately what I am wanting to do with this in our local setup is to not use this new functionality and have full backwards compatibility with how we use this service today (no remote writes and no loki integration for events), it's not clear to me if your changes break that or if there are new options I need to leverage to preserve this. Note, we just went through a staffing refresh here, so I am hoping to know have a developer do a sanity test on this for backwards compatibility but would like to get some input from you before doing so.

If its easier to do a quick video chat on these changes to better understand topics like the decision tree usage, please let me know and I can setup.

thanks -

@dubyte
Copy link
Author

dubyte commented Mar 24, 2022

Hi @ober37:

Yes the intention of the PR is to keep the compatibility of the project.
The feature to send the metrics using remote write is disable by default,
And the option to disable the metrics from the api is off too.

About logs, no wories the logs are being written in stdout.
what we is being sending to loki is a an especial case of sparkplug mesg. And to know what messages should end in metrics and what msgs should end in Loki we use a decision tree.

The reason that Loki is not optional I because I was expecting to finish to check with you first the prometheus remote write first.

But now that I submit all let me make all new stuff optional and I will check that the project continues working as it was working without any breaking changes.

But yeah lets make a video call and we can talk about the changes.

Thanks let's continue the dialogue.

@xkilian
Copy link

xkilian commented Apr 28, 2022

Hi,@ober37, I believe the modifications you asked for have been done. Can you continu your review and testing to be able to integrate the changes.

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