isomerpages / isomercms-backend

A static website builder and host for the Singapore Government
5 stars 1 forks source link

Improve APM spans (no more "<anonymous>"!) #1266

Closed timotheeg closed 6 months ago

timotheeg commented 6 months ago

Problem

We have a lot of spans in APM traces which are <anonymous>. While not a deal breaker to get value out of APM, it is annoying.

image

The anonymous functions are due to the way some methods are declared in classes, which is done to be able to specify types.

e.g. in ReviewsRouter, the assignment syntax creates anonymous methods

  compareDiff: RequestHandler<
    { siteName: string },
    { items: EditedItemDto[] } | ResponseErrorBody,
    unknown,
    unknown,
    { userWithSiteSessionData: UserWithSiteSessionData }
  > = async (req, res) => {
    // ... 
  }

while in collectionPages, this style of named method works:

  async updateCollectionPage(req, res, next) {
    // ...
  }

Note: The first style of method declaration does NOT create methods on the class' prototype. The methods are created on the instance themselves at construction (and so just FYI the call to autoBind(this) are actually useless for classes which only use that style of declaration)

TODO: We should ensure that all methods we use as express route handlers are named to have nice usable traces

Solution

Updating the Router classes was done via the script below, which was then cleaned up on commit by eslint/prettier:

grep 'autoBind(this)' $(git grep Router | grep class | grep Router | cut -d: -f1) | cut -d: -f1 | while read f; do
  echo 'import { nameAnonymousMethods } from "@root/utils/apm-utils"' > temp;
  sed -E -e 's/autoBind\(this\)/nameAnonymousMethods(this);autoBind(this)/' "${f}" >> temp;
  mv temp "${f}";
done

Verified on staging: image

Tests