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

count dispatches #3

Merged
merged 3 commits into from
May 7, 2021
Merged

count dispatches #3

merged 3 commits into from
May 7, 2021

Conversation

lieut-data
Copy link
Member

Summary

Keep track of when the plugin started and the number of dispatches that have occurred since that time. Include these stats, the current time, and the computed number of dispatches per second in the stats emitted by window.logStoreStatistics.

Keep track of when the plugin started and the number of dispatches that have occurred since that time. Include these stats, the current time, and the computed number of dispatches per second in the stats emitted by `window.logStoreStatistics`.
@@ -7,16 +7,25 @@ import manifest from './manifest';
// eslint-disable-next-line import/no-unresolved
import {PluginRegistry} from './types/mattermost-webapp';

const globalStats = {
Start: Date.now(),
NumDispatches: 0,
Copy link
Member Author

Choose a reason for hiding this comment

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

This is very coarse -- a better solution might be to track in buckets of time, so we can correlate perceived performance with the number of dispatches observed near that unit of time. But this might be good enough as a baseline.

Copy link
Member

Choose a reason for hiding this comment

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

We could also add something to count which actions were dispatched the most too

@jwilander
Copy link
Member

Might want to bump the plugin version actually

@lieut-data
Copy link
Member Author

Oh, right! I'm used to the incident collaboration workflow, which no longer requires managing plugin.json :p -- need to port that back to starter-template at some point.

export default class Plugin {
// eslint-disable-next-line @typescript-eslint/no-unused-vars, @typescript-eslint/no-empty-function
public async initialize(registry: PluginRegistry, store: Store<GlobalState, Action<Record<string, unknown>>>) {
window.webappDebugStore = store;
store.subscribe(() => {
Copy link
Member

Choose a reason for hiding this comment

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

Should we unsubscribe in case we update the plugin afterwards? I don't know how much we care about leaking these subscriptions

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, I can add that -- though deinitialize isn't "reliable". It works if you explicitly disable a plugin, but not if you just upload a replacement. I don't think the leak here will matter (compared to the issue being debugged at present!), but we could always give explicit instructions.

@lieut-data lieut-data requested a review from hmhealey May 7, 2021 19:44
@hmhealey hmhealey added the 4: Reviews Complete All reviewers have approved the pull request label May 7, 2021
@lieut-data lieut-data merged commit 8dd432e into master May 7, 2021
@jwilander jwilander deleted the count-dispatches branch May 7, 2021 20:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants