Plugin Support + New Statistic Plugins

[darrenpicard25]

Added new statisticsCollected and statisticsCollectedError events to processors as well as new configuration option. Added new statistics method to datastore

This work is to allow better monitoring capabilities to chrono.

Changes are considered breaking due to the forced change on the Datastore interface. As we are still pre 1.0 release I am okay forcing changes

Additional Things:

• Improved some documentation
• added more tests
• updated dev dependencies
• set up lint-staged
• improved CI pipeline

[wiz-a38520980d]

Wiz Scan Summary

Scanner	Findings
Vulnerabilities	5  2
Sensitive Data	-
Secrets	-
IaC Misconfigurations	-
SAST Findings	-
Total	5  2

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension.

[darrenpicard25]

something i missed from my previous update supporting esm and commonjs

[tjamescouch]

Seems a little odd to have a function not named using a verb. Any reason not to name it getStatistics?

[darrenpicard25]

agree it is weird. i originally had getStatistics but it also kinda looked wrong next to claim, delete, retry, schedule, etc

but maybe that is just me. let me know your preference

[tjamescouch]

TBH I grew to like it by the end of reading. Im ok either way, the brevity is nice even if it does not meet style guides :)

[maiahneo]

The statistics method may seem fine. Another option we can use here is collectStatistics(...). What do you think?

[neo-luca]

It would be amazing to also get stats for scheduled tasks, no?

[darrenpicard25]

not sure im following you here.

[maiahneo]

Looks fine. But I'm thinking of doing the Statistics outside of SimpleProcessor. The SimpleProcessor should be just about processing the tasks and it's lifecycle.

Maybe we can have a design like:

class Chrono {
  // Other exiting methods  here...

  //  This is an existing method
  public registerTaskHandler() {
    const processor = createProcessor({...});
    const statsCollector = createStatsCollector({ processor });

    // then we can return the collector here as well and the
    // user can listen from its events
    return { processor, statsCollector } 
}

Or another approach is to put the StatsCollector outside Chrono so we can do

const chrono = new Chrono();
const statsCollector = new StatsCollector({ chrono });

// then maybe we can start it...
statsCollector.start()

// then listen to its events for example...
statsCollector.on('stats-collected', () => { ... });
statsCollector.on('stats-collection-error', () => { ... });

Obviously those are just pseudo(s) but the idea is to de-couple the Stats collector from the Task Processor in hope to make those 2 distinct processes simpler, easier to maintain, and debug in the future. Let me know your thoughts! 😉

[maiahneo]

😅

[maiahneo]

Agree on James' concern on the naming of the method below. I have a suggestion but non-blocking since it's an internal method (for data-stores to implement) and we can change in the future.

[maiahneo]

The statistics method may seem fine. Another option we can use here is collectStatistics(...). What do you think?

[darrenpicard25]

Looks fine. But I'm thinking of doing the Statistics outside of SimpleProcessor. The SimpleProcessor should be just about processing the tasks and it's lifecycle.

Maybe we can have a design like:

class Chrono {
  // Other exiting methods  here...

  //  This is an existing method
  public registerTaskHandler() {
    const processor = createProcessor({...});
    const statsCollector = createStatsCollector({ processor });

    // then we can return the collector here as well and the
    // user can listen from its events
    return { processor, statsCollector } 
}

Or another approach is to put the StatsCollector outside Chrono so we can do

const chrono = new Chrono();
const statsCollector = new StatsCollector({ chrono });

// then maybe we can start it...
statsCollector.start()

// then listen to its events for example...
statsCollector.on('stats-collected', () => { ... });
statsCollector.on('stats-collection-error', () => { ... });

Obviously those are just pseudo(s) but the idea is to de-couple the Stats collector from the Task Processor in hope to make those 2 distinct processes simpler, easier to maintain, and debug in the future. Let me know your thoughts! 😉

damn..... that stat collector is a pretty good idea

[maiahneo]

Great work on this Darren! I like the Plugins idea 😃

There's a lot of things going on in this PR. To review it properly do you it can be break down to parts like below?

PR 1: Code gardening, docs update, config updates, npm deps updates.
PR 2: Chrono plugin support.
PR 3: Statistics collector.

[maiahneo]

Thanks for adding this Darren! ❤️

[maiahneo]

I like the chrono.use(myPlugin) syntax! It feels convenient and lightweight like how express/hono middleware does.

💅 Just to make it straight forward (API seems vague when I read it, and I need to read the jsdoc to understand what it represents)

Suggested change

	use<API>(plugin: ChronoPlugin<TaskMapping, API>): API {
	use<PluginAPI>(plugin: ChronoPlugin<TaskMapping, PluginAPI>): PluginAPI {