In order to resolve this task, I need review on the following three things:

Java implementation notes

Compare this description of the Java implementation with the current description. Is there anything from the former that should be moved into the latter before I redirect Metrics_Platform/Client/Implementations?

Automated vs manual docs strategy

The only automated docs I was able to find for the MP clients are https://doc.wikimedia.org/EventLogging/master/js/MediaWikiMetricsClientIntegration.html, but these docs seem to cover only the internal methods, not the public methods. Is that the intention? Are there are plans for these docs or any other automated client library docs I should be aware of?

In terms of how we should document the public methods, I think in this case it would be easier to document the public methods manually on Wikitech instead of spinning up four automated docs sites, one for each language. I expect there will be roughly fewer than 10 public methods per client, so it should be manageable to maintain the docs manually. Having everything on wiki should also make it easier to keep the usage examples up to date. Let me know what you think!

Development TODOs

Metrics Platform/Client/Implementations includes several TODOs within the page. Since I'll be redirecting this page, I've listed these TODOs here in case the team wants to turn them into tasks. (In general, TODOs in docs should only be for the docs themselves, not the code.)

JavaScript:

• Currently, a number of other methods are attached to the core object and thereby exported via mw.eventLog. These include streamInSample(), and all methods under mw.eventLog.id and mw.eventLog.storage. These should not be invoked by external callers and will be removed from the de facto public interface in a future refactor.

PHP:

• For the sake of cross-platform consistency, the submit() method should be updated to remove the optional $logger parameter. This will probably require restoring the standalone EventPlatformClient class and injecting a logger in the class constructor.

Java:

• For consistency with the Swift library, add an InputBuffer to temporarily retain events that occur before stream configs first become available.
• Update public method sig to: submit(String name, Event data)

Swift:

• Update clients to pass in the intended domain value as meta.domain, and remove the optional domain parameter.

In T372682#10173644, @apaskulin wrote:

Java implementation notes

<snip />

The new version LGTM!

Automated vs manual docs strategy

The only automated docs I was able to find for the MP clients are https://doc.wikimedia.org/EventLogging/master/js/MediaWikiMetricsClientIntegration.html, but these docs seem to cover only the internal methods, not the public methods. Is that the intention? Are there are plans for these docs or any other automated client library docs I should be aware of?

I think that this is a result of the JS MPC using TypeScript for type checking. EventLogging uses JSDoc to generate its docs. JSDoc tends to warn (best case) or error (worst case) when parsing TypeScript types and so we exclude the JS MPC source files in the EventLogging JSDoc config (many times, apparently…)

I've submitted an MR to migrate the JS MPC to use JSDoc and update its documentation so that: 1) more of its documentation can be generated automatically; and 2) that documentation can be included in the EventLogging docs. The MR is a draft, needs a description, and should probably have its own Phab task.

In terms of how we should document the public methods, I think in this case it would be easier to document the public methods manually on Wikitech instead of spinning up four automated docs sites, one for each language. I expect there will be roughly fewer than 10 public methods per client, so it should be manageable to maintain the docs manually. Having everything on wiki should also make it easier to keep the usage examples up to date. Let me know what you think!

I defer to your judgement as I'm unsure.

Development TODOs

JavaScript:

• Currently, a number of other methods are attached to the core object and thereby exported via mw.eventLog. These include streamInSample(), and all methods under mw.eventLog.id and mw.eventLog.storage. These should not be invoked by external callers and will be removed from the de facto public interface in a future refactor.

PHP:

• For the sake of cross-platform consistency, the submit() method should be updated to remove the optional $logger parameter. This will probably require restoring the standalone EventPlatformClient class and injecting a logger in the class constructor.

Swift:

• Update clients to pass in the intended domain value as meta.domain, and remove the optional domain parameter.

These should be tasks. I'll create 'em.

Java:

• For consistency with the Swift library, add an InputBuffer to temporarily retain events that occur before stream configs first become available.
• Update public method sig to: submit(String name, Event data)

These are both done 🎉

Thanks, @phuedx! This all works for me.

I've submitted an MR to migrate the JS MPC to use JSDoc and update its documentation so that: 1) more of its documentation can be generated automatically; and 2) that documentation can be included in the EventLogging docs. The MR is a draft, needs a description, and should probably have its own Phab task.

Sounds great! I'm happy to review the JSDoc output. At first glance, I'm not sure how to test the output of the MR prior to being merged and updated in EventLogging, so my review may need to wait for that unless the process is relatively simple.

Regarding the choice between automated vs manual docs, since we already have a publishing pipeline set up for the JSDocs, I think it makes sense to replace the manual docs for the JS client with the JSDoc site once it's ready. For the other client languages, we can start with manual docs, and then move to automated if we choose to prioritize setting up automated docs sites for those languages.

In T372682#10200312, @apaskulin wrote:

I've submitted an MR to migrate the JS MPC to use JSDoc and update its documentation so that: 1) more of its documentation can be generated automatically; and 2) that documentation can be included in the EventLogging docs. The MR is a draft, needs a description, and should probably have its own Phab task.

Sounds great! I'm happy to review the JSDoc output. At first glance, I'm not sure how to test the output of the MR prior to being merged and updated in EventLogging, so my review may need to wait for that unless the process is relatively simple.

You should be able to run something close to the following locally:

cd /path/to/metrics-platform
git fetch origin
git checkout -b 'jsdoc' 'origin/jsdoc'
cd js
npm run doc
open docs/@wikimedia/metrics-platform/0.1.0/index.html

Regarding the choice between automated vs manual docs, since we already have a publishing pipeline set up for the JSDocs, I think it makes sense to replace the manual docs for the JS client with the JSDoc site once it's ready. For the other client languages, we can start with manual docs, and then move to automated if we choose to prioritize setting up automated docs sites for those languages.

👍

Being bold and moving to Done. Feel free to revert if sign off is needed