alertmanager
alertmanager copied to clipboard
Enable setting ThreadId for Telegram notifications
Enable setting message_thread_id for Telegram notifications.
Also see https://github.com/grafana/grafana/pull/79546.
Please add
Also, please update the description of this PR with more information. Link to another PR is not a really good description. Thanks!
Please add
Also, please update the description of this PR with more information. Link to another PR is not a really good description. Thanks!
Done 👍
The tests looked OK to me, but let me know if you think you need more cases.
Any updates?
@gotjosh @simonpasquier can you please review the PR?
@yuri-tceretian This is a duplicate of an existing pull request, https://github.com/prometheus/alertmanager/pull/3560, which has been pending review since October 16, 2023, nearing four months. Several maintainers have been tagged for attention. It is disappointing to see this duplicate receiving more attention.
@yuri-tceretian This is a duplicate of an existing pull request, #3638, which has been pending review since October 16, 2023, nearing four months. Several maintainers have been tagged for attention. It is disappointing to see this duplicate receiving more attention.
If you mean https://github.com/prometheus/alertmanager/pull/3560, this is not duplicate, message_thread_id inclusion is different compared to reply_to_message_id, even though the idea is the same (sending message to a certain topic), message_thread is the intended way to sending a message to a certain topic, reply_to_message, as the fields name says, are more targeted to replying to a certain message ID rather than sending messages.
@yuri-tceretian This is a duplicate of an existing pull request, #3638, which has been pending review since October 16, 2023, nearing four months. Several maintainers have been tagged for attention. It is disappointing to see this duplicate receiving more attention.
If you mean #3560, this is not duplicate, message_thread_id inclusion is different compared to reply_to_message_id, even though the idea is the same (sending message to a certain topic), message_thread is the intended way to sending a message to a certain topic, reply_to_message, as the fields name says, are more targeted to replying to a certain message ID rather than sending messages.
Thank you, @raphielscape, for explaining the differences between PR #3560 and the current request. Your insight into how message_thread_id differs from reply_to_message_id is very helpful. Considering both PRs are focused on enhancing Telegram-related functionalities, it might be beneficial if the maintainers could review and possibly merge them both 🚀
hi guys, just a quick question. any plan for merging this PR and releasing the new feature?
Please rebase and sign. See https://github.com/prometheus/alertmanager/pull/3638/checks?check_run_id=21162278935
Please rebase and sign. See https://github.com/prometheus/alertmanager/pull/3638/checks?check_run_id=21162278935
I followed the instructions on the link you shared, please let me know if there is anything else I need to do 💐
@th0th I don't think you rebased correctly as there are now 73 commits in this PR. Did you use git rebase? You also still need to sign off all of your commits with --signoff.
@th0th I don't think you rebased correctly as there are now 73 commits in this PR. Did you use
git rebase? You also still need to sign off all of your commits with--signoff.
I followed the exact instructions on the link, but I think the commit history got messed up since I previously merged the upstream branch to my own branch. And now I don't know how to fix 😅
You can squash all the commits into a single commit using git rebase and then sign the commit using git commit --amend --no-edit --signoff.
Another option is to delete the branch, create a new branch with the same name, add the modifications, commit them, and then force push the branch to GitHub and it will replace the old branch.
You can squash all the commits into a single commit using
git rebaseand then sign the commit usinggit commit --amend --no-edit --signoff.Another option is to delete the branch, create a new branch with the same name, add the modifications, commit them, and then force push the branch to GitHub and it will replace the old branch.
I rebased, and it reduced the number of commits but there are still some commits that are shown in this PR, but not mine. No idea why.
The compile fail was due to a typo I guess, I fixed that, too.
I rebased, and it reduced the number of commits but there are still some commits that are shown in this PR, but not mine. No idea why.
Can you share the commands you ran to rebase?
Can you also run go mod tidy as otherwise it doesn't seem to compile:
../../../../../../pkg/mod/github.com/go-openapi/[email protected]/header.go:21:2: missing go.sum entry for module providing package github.com/go-openapi/jsonpointer (imported by github.com/go-openapi/analysis); to add:
go get github.com/go-openapi/[email protected]
../../../../../../pkg/mod/github.com/go-openapi/[email protected]/ref.go:25:2: missing go.sum entry for module providing package github.com/go-openapi/jsonreference (imported by github.com/go-openapi/spec); to add:
go get github.com/go-openapi/[email protected]
I rebased, and it reduced the number of commits but there are still some commits that are shown in this PR, but not mine. No idea why.
Can you share the commands you ran to rebase?
I did this:
git remote add upstream https://github.com/prometheus/alertmanager.git
git rebase upstream/main
I rebased, and it reduced the number of commits but there are still some commits that are shown in this PR, but not mine. No idea why.
Can you share the commands you ran to rebase?
I did this:
git remote add upstream https://github.com/prometheus/alertmanager.git git rebase upstream/main
Did you merge main before the rebase? If so, that might have caused the issue. I would just squash all 16 commits in this branch to a single commit using git rebase and then we can go from there. There is a nice tutorial on how to do that here.
come on :))) merge this
Hi @th0th! 👋 Do you need help with the rebase?
Hi @th0th! 👋 Do you need help with the rebase?
I do indeed 🙃
OK. I ran the following:
git fetch
git rebase origin/main // here origin is [email protected]:prometheus/alertmanager.git
git rebase -i HEAD~16
I then did the following:
pick 9f7f92cd Enable setting ThreadId for telegram notifications
s 7dceb6fa Fix the typo
s 0274e497 Update tests
s 5ec55ae7 Update documentation
s 15682c4a Update notifiers_test.go
s a75dd888 Update notifiers_test.go
s b6017dd0 Use int32 instead of int64 for MessageThreadID
s 3cecaae6 Bump github.com/go-openapi/swag from 0.22.4 to 0.22.7 (#3655)
d b29d9af6 Support UTF-8 label matchers: Add metrics to matchers compat package (#3658)
d c8eca8d9 Change compat metrics to counters (#3686)
s 3c958067 Bump github.com/go-openapi/errors from 0.20.4 to 0.21.0
s aeb3cdbd Bump github.com/go-openapi/swag from 0.22.7 to 0.22.9
s 37426b93 Bump eslint from 8.35.0 to 8.56.0 in /ui/react-app (#3692)
d 084e0d0c Do not register compat metrics in amtool (#3713)
d 70540688 Remove metrics from compat package (#3714)
s ef146bce Fix the typo
This removes the unrelated commits and squashes the others into a single commit. I also removed the commit messages of all other commits except the first commit. You can see here that this leaves just one commit ahead of main:
git log master..th0th/main --oneline
fa78ab4b (HEAD -> th0th/main) Enable setting ThreadId for telegram notifications
cb9724db (origin/main, origin/HEAD, main) notify/webhook: Fix crash on errors when url_file is used (#3798) (#3800)
OK. I ran the following:
git fetch git rebase origin/main // here origin is [email protected]:prometheus/alertmanager.git git rebase -i HEAD~16I then did the following:
pick 9f7f92cd Enable setting ThreadId for telegram notifications s 7dceb6fa Fix the typo s 0274e497 Update tests s 5ec55ae7 Update documentation s 15682c4a Update notifiers_test.go s a75dd888 Update notifiers_test.go s b6017dd0 Use int32 instead of int64 for MessageThreadID s 3cecaae6 Bump github.com/go-openapi/swag from 0.22.4 to 0.22.7 (#3655) d b29d9af6 Support UTF-8 label matchers: Add metrics to matchers compat package (#3658) d c8eca8d9 Change compat metrics to counters (#3686) s 3c958067 Bump github.com/go-openapi/errors from 0.20.4 to 0.21.0 s aeb3cdbd Bump github.com/go-openapi/swag from 0.22.7 to 0.22.9 s 37426b93 Bump eslint from 8.35.0 to 8.56.0 in /ui/react-app (#3692) d 084e0d0c Do not register compat metrics in amtool (#3713) d 70540688 Remove metrics from compat package (#3714) s ef146bce Fix the typoThis removes the unrelated commits and squashes the others into a single commit. I also removed the commit messages of all other commits except the first commit. You can see here that this leaves just one commit ahead of
main:git log master..th0th/main --oneline fa78ab4b (HEAD -> th0th/main) Enable setting ThreadId for telegram notifications cb9724db (origin/main, origin/HEAD, main) notify/webhook: Fix crash on errors when url_file is used (#3798) (#3800)
Thanks a lot @grobinson-grafana. But when I run git rebase -i HEAD~16, and put in the input, it says:
➜ alertmanager git:(main) git rebase -i HEAD~16
error: could not parse '9f7f92cd'
error: invalid line 1: pick 9f7f92cd Enable setting ThreadId for telegram notifications
error: could not parse '7dceb6fa'
error: invalid line 2: s 7dceb6fa Fix the typo
error: could not parse '0274e497'
error: invalid line 3: s 0274e497 Update tests
error: could not parse '5ec55ae7'
error: invalid line 4: s 5ec55ae7 Update documentation
error: could not parse '15682c4a'
error: invalid line 5: s 15682c4a Update notifiers_test.go
error: could not parse 'a75dd888'
error: invalid line 6: s a75dd888 Update notifiers_test.go
error: could not parse 'b6017dd0'
error: invalid line 7: s b6017dd0 Use int32 instead of int64 for MessageThreadID
error: could not parse '3cecaae6'
error: invalid line 8: s 3cecaae6 Bump github.com/go-openapi/swag from 0.22.4 to 0.22.7 (#3655)
error: could not parse 'b29d9af6'
error: invalid line 9: d b29d9af6 Support UTF-8 label matchers: Add metrics to matchers compat package (#3658)
error: could not parse 'c8eca8d9'
error: invalid line 10: d c8eca8d9 Change compat metrics to counters (#3686)
error: could not parse '3c958067'
error: invalid line 11: s 3c958067 Bump github.com/go-openapi/errors from 0.20.4 to 0.21.0
error: could not parse 'aeb3cdbd'
error: invalid line 12: s aeb3cdbd Bump github.com/go-openapi/swag from 0.22.7 to 0.22.9
error: could not parse '37426b93'
error: invalid line 13: s 37426b93 Bump eslint from 8.35.0 to 8.56.0 in /ui/react-app (#3692)
error: could not parse '084e0d0c'
error: invalid line 14: d 084e0d0c Do not register compat metrics in amtool (#3713)
error: could not parse '70540688'
error: invalid line 15: d 70540688 Remove metrics from compat package (#3714)
error: could not parse 'ef146bce'
error: invalid line 16: s ef146bce Fix the typo
You can fix this with 'git rebase --edit-todo' and then run 'git rebase --continue'.
Or you can abort the rebase with 'git rebase --abort'.
What am I doing wrong? :(
What am I doing wrong? :(
You should be able to edit what is there when you run git rebase -i HEAD~16 – you don't need to copy/paste what I wrote. Basically you should change all pick to s (for squash) with the following exceptions:
9f7f92cdshould be kept aspickbecause this is the first commitb29d9af6should bed(for drop)c8eca8d9should bed(for drop)084e0d0cshould bed(for drop)70540688should bed(for drop)
What am I doing wrong? :(
You should be able to edit what is there when you run
git rebase -i HEAD~16– you don't need to copy/paste what I wrote. Basically you should change allpicktos(for squash) with the following exceptions:
9f7f92cdshould be kept aspickbecause this is the first commitb29d9af6should bed(for drop)c8eca8d9should bed(for drop)084e0d0cshould bed(for drop)70540688should bed(for drop)
I think I managed to rebase, can you please confirm @grobinson-grafana? 💐
I think I managed to rebase, can you please confirm @grobinson-grafana? 💐
Looks good to me! @gotjosh can you help with CircleCI?
Thanks Josh! @th0th there are some failing tests (please see CI / Test https://github.com/prometheus/alertmanager/actions/runs/8892559543/job/24416875284?pr=3638). You can also run make test before committing the fix to make sure there are no other failing tests.
Thank you very much for your contribution!
@th0th Thanks also for the contribution! I would appreciate it if you could test it and make sure it works as I cannot seem to create a Telegram supergroup or topics (perhaps I just don't know how to do it?).
Thanks for the help getting this merged folks :) @grobinson-grafana I would love to test, but I am using grafana right now, and I don't have alertmanager running separately :(