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

[MM-63254] Add diagnostics data to the Support Packet #445

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

Conversation

hanzei
Copy link
Collaborator

@hanzei hanzei commented Feb 25, 2025

Summary

When a Support Packet gets generated, the plugin now includes diagnostics data via a diagnostics.yaml file into the Packet.

Example content of com.mattermost.mscalendar/diagnostics.yaml:

version: 1.3.4
connected_user_count: 2
subscription_count: 5
is_oauth_configured: true

Requires mattermost/mattermost#28833 on the server side.

See https://mattermost.atlassian.net/wiki/spaces/CLOUD/pages/3083108354/Diagnostics+data+from+Core+Plugins+to+the+Support+Packet for more context.

Ticket Link

https://mattermost.atlassian.net/browse/MM-63254

@hanzei hanzei added the 2: Dev Review Requires review by a core committer label Feb 25, 2025
@hanzei hanzei changed the title [MM-61118] Add diagnostics data to the Support Packet [MM-63254] Add diagnostics data to the Support Packet Feb 25, 2025
@hanzei hanzei marked this pull request as ready for review February 25, 2025 09:48
@hanzei hanzei requested a review from wiggin77 as a code owner February 25, 2025 09:48
@hanzei hanzei added the 3: QA Review Requires review by a QA tester label Feb 25, 2025
@hanzei hanzei requested a review from Copilot February 27, 2025 16:02

Choose a reason for hiding this comment

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

PR Overview

This PR adds a diagnostics.yaml file to the Support Packet generated by the plugin, providing useful diagnostic information such as plugin version, connected user count, subscription count, and OAuth configuration status.

  • Adds a new SupportPacket type and GenerateSupportData function in calendar/plugin/support_packet.go.
  • Introduces GetSubscriptionCount and GetConnectedUserCount functions in the store layers and updates corresponding tests and mocks.
  • Updates several test cases to use pointer-based user identifiers for consistency.

Reviewed Changes

File Description
calendar/plugin/support_packet.go Implements diagnostics data generation for the Support Packet.
calendar/store/subscription_store.go Adds function to count subscriptions using paginated KVList calls.
calendar/store/user_store.go Adds function to count connected users using paginated KVList calls.
calendar/utils/kvstore/*.go Extends the KVStore interface with the List method and updates consumers.
Various test files (subscription_store_test.go, user_store_test.go, etc.) Update tests to cover new counting functions and pointer-based user IDs.
calendar/config/config.go Adds IsOAuthConfigured method to StoredConfig.
calendar/engine/*_test.go Updates mock user creation to use pointer types.

Copilot reviewed 23 out of 23 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (1)

calendar/plugin/support_packet.go:50

  • [nitpick] Clarify the usage of p.env.PluginVersion as the directory component in the filename; if this is not intended, consider using a dedicated constant or a more descriptive name for the diagnostics file path to improve clarity.
Filename: filepath.Join(p.env.PluginVersion, "diagnostics.yaml")
Copy link
Member

@wiggin77 wiggin77 left a comment

Choose a reason for hiding this comment

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

LGTM 👍 The KVStore ought to have some counting methods. Fetching all the keys just to count them seems inefficient. Some customers will require dozens of database hits just to count the keys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants