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.

Sign up for GitHub

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

Jump to bottom

Update prometheus message format #966

Merged
merged 1 commit into from
Feb 3, 2023
Merged

Update prometheus message format #966

merged 1 commit into from
Feb 3, 2023

Conversation

mszostok
Copy link
Contributor

@mszostok mszostok commented Feb 2, 2023

Description

Changes proposed in this pull request:

  • Update prometheus message format

@mszostok mszostok added the enhancement New feature or request label Feb 2, 2023
@huseyinbabal
Copy link
Contributor

@mszostok tested locally, and it works 🚀
Screen Shot 2023-02-03 at 10 02 33

pkosiec
pkosiec reviewed Feb 3, 2023
View reviewed changes
internal/source/prometheus/source.go
Comment on lines +78 to +82
Source: %s
Alert Name: %s
State: %s

Description:
Copy link
Member

Choose a reason for hiding this comment

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

What would happen if we used bold for keys? Like:

Suggested change
Source: %s
Alert Name: %s
State: %s
Description:
*Source:* %s
*Alert Name:* %s
*State:* %s
*Description:*

I suspect you already thought of this, and such formatting is not handled correctly in all comm platforms, correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

@pkosiec it becomes bold
Screen Shot 2023-02-03 at 10 49 22

Copy link
Member

Choose a reason for hiding this comment

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

On Slack and probably Discord + Mattermost yes, but not sure about MS Teams 🤔 I know @mszostok knows a few caveats about MS Teams, that's why I asked 😄 But hopefully we can apply the bold 🤞

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So the problem is that Slack uses single * where others uses double ** for bold statements :( only ` is common.

@mszostok mszostok marked this pull request as ready for review February 3, 2023 09:04
@mszostok mszostok requested review from a team and PrasadG193 as code owners February 3, 2023 09:04
@mszostok
Copy link
Contributor Author

mszostok commented Feb 3, 2023

Thanks guys, I answered your comments, please take a look once again :)

@pkosiec @huseyinbabal

@mszostok
Copy link
Contributor Author

mszostok commented Feb 3, 2023 •
edited
Loading

Only MS Teams doesn't respect properly new lines:
Screenshot 2023-02-03 at 10 48 55
but it's still quite readable.

@mszostok mszostok merged commit 47152d8 into kubeshop:main Feb 3, 2023
@mszostok mszostok deleted the update-prometheus branch February 3, 2023 10:39
mszostok added a commit that referenced this pull request Feb 3, 2023
@mszostok
Cherry-pick fixes and enhancements into 0.18 (#969)
ff8667d 
* Update Prometheus message format (#966)

* Add SourceBindings built-in alias and fix filtering commands (#967)

* Use new format to download deps, remove deprecated DownloadDependencies method  (#968)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pkosiec pkosiec pkosiec left review comments

@huseyinbabal huseyinbabal huseyinbabal approved these changes

@PrasadG193 PrasadG193 Awaiting requested review from PrasadG193 PrasadG193 is a code owner

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mszostok @huseyinbabal @pkosiec