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

[MI-3624] Add some additional metrics #354

Merged
merged 13 commits into from
Oct 24, 2023
Merged

[MI-3624] Add some additional metrics #354

merged 13 commits into from
Oct 24, 2023

Conversation

ayusht2810
Copy link
Contributor

@ayusht2810 ayusht2810 commented Oct 18, 2023 •
edited
Loading

Summary

  • Added metrics for messages, reactions and files count for different sources and actions.
  • Fixed unit test cases

@ayusht2810 ayusht2810 self-assigned this Oct 18, 2023
@ayusht2810 ayusht2810 added Difficulty/3:Hard Hard ticket Test Cases Includes unit test cases changes labels Oct 18, 2023
raghavaggarwal2308
raghavaggarwal2308 suggested changes Oct 18, 2023
View reviewed changes
server/handlers/handlers.go Outdated Show resolved Hide resolved
server/handlers/handlers.go Show resolved Hide resolved
server/message_hooks.go Outdated Show resolved Hide resolved
Kshitij-Katiyar
Kshitij-Katiyar reviewed Oct 23, 2023
View reviewed changes
server/handlers/attachments.go Outdated Show resolved Hide resolved
server/handlers/attachments.go Outdated Show resolved Hide resolved
server/handlers/handlers.go Outdated Show resolved Hide resolved
server/handlers/handlers.go Outdated Show resolved Hide resolved
server/message_hooks.go Outdated Show resolved Hide resolved
server/metrics/metrics.go Outdated Show resolved Hide resolved
server/plugin.go Show resolved Hide resolved
server/plugin.go Outdated Show resolved Hide resolved
@ayusht2810 ayusht2810 mentioned this pull request Oct 23, 2023
Merged
manojmalik20
manojmalik20 reviewed Oct 23, 2023
View reviewed changes
server/handlers/attachments.go Outdated Show resolved Hide resolved
server/handlers/handlers.go Outdated Show resolved Hide resolved
server/handlers/handlers.go Outdated Show resolved Hide resolved
server/message_hooks.go Outdated Show resolved Hide resolved
lieut-data
lieut-data requested changes Oct 23, 2023
View reviewed changes
Copy link
Member

@lieut-data lieut-data left a comment

Choose a reason for hiding this comment

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

Thanks, @ayusht2810! Just giving a quick preliminary review as it looks like there's already some feedback to address.

server/handlers/handlers.go Outdated Show resolved Hide resolved
server/metrics/metrics.go
@@ -16,12 +16,31 @@ const (
MetricsCloudInstallationLabel = "installationId"
)

type Metrics interface {
Copy link
Member

Choose a reason for hiding this comment

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

Looks like multiple PRs are all touching this interface -- lots of merge conflicts ahead!

@ayusht2810 ayusht2810 mentioned this pull request Oct 24, 2023
Merged
lieut-data
lieut-data approved these changes Oct 24, 2023
View reviewed changes
Copy link
Member

@lieut-data lieut-data left a comment

Choose a reason for hiding this comment

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

Thanks, @ayusht2810! A few comments, but not strictly blocking.

server/handlers/attachments.go Outdated Show resolved Hide resolved
server/metrics/metrics.go Show resolved Hide resolved
@manojmalik20
[MI-3624] Review fixes: Added another metric function for observing f…
2e0bfec 
…iles count and renamed a variable

Fixed failing unit tests
@manojmalik20 manojmalik20 merged commit 211d50a into main Oct 24, 2023
1 check passed
@manojmalik20 manojmalik20 deleted the MI-3624 branch October 24, 2023 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kshitij-Katiyar Kshitij-Katiyar Kshitij-Katiyar approved these changes

@manojmalik20 manojmalik20 manojmalik20 approved these changes

@raghavaggarwal2308 raghavaggarwal2308 raghavaggarwal2308 approved these changes

@lieut-data lieut-data lieut-data approved these changes

Assignees

@ayusht2810 ayusht2810

Labels
Difficulty/3:Hard Hard ticket Test Cases Includes unit test cases changes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ayusht2810 @lieut-data @raghavaggarwal2308 @manojmalik20 @Kshitij-Katiyar