Improve APM spans (no more <anonymous>) (#1267)

* refactor: rename wrapper based on original function, only when original.name exists

* feat: utility to name all methods of an object

* feat: ensure all route handlers are named

* feat: drop the bound prefix in span names

* chore: remove unused eslint rule disabling

---------

Co-authored-by: Alexander Lee <[email protected]>

src/middleware/routeHandler.ts

@@ -83,11 +83,16 @@ const nameRouteHandlerWrapper = <
   Params = Record<string, unknown>,
   Locals extends Record<string, unknown> = Record<string, unknown>
 >(
-  func: RequestHandler<Params, ResBody, ReqBody, ReqQuery, Locals>,
-  name: string
+  wrapper: RequestHandler<Params, ResBody, ReqBody, ReqQuery, Locals>,
+  original: { name: string }
 ): RequestHandler<Params, ResBody, ReqBody, ReqQuery, Locals> => {
-  Object.defineProperty(func, "name", { value: name, writable: false })
-  return func
+  if (original.name) {
+    Object.defineProperty(wrapper, "name", {
+      value: original.name.replace(/^bound /, ""),
+      writable: false,
+    })
+  }
+  return wrapper
 }
 
 // Used when there are no write API calls to the repo on GitHub
@@ -96,7 +101,7 @@ export const attachReadRouteHandlerWrapper: RouteWrapper = (routeHandler) =>
     Promise.resolve(routeHandler(req, res, next)).catch((err: Error) => {
       next(err)
     })
-  }, routeHandler.name)
+  }, routeHandler)
 
 // Used when there are write API calls to the repo on GitHub
 export const attachWriteRouteHandlerWrapper: RouteWrapper<{
@@ -135,7 +140,7 @@ export const attachWriteRouteHandlerWrapper: RouteWrapper<{
         next(err)
       }
     )
-  }, routeHandler.name)
+  }, routeHandler)
 
 export const attachRollbackRouteHandlerWrapper: RouteWrapper<
   {
@@ -340,4 +345,4 @@ export const attachRollbackRouteHandlerWrapper: RouteWrapper<
         next(err)
       }
     )
-  }, routeHandler.name)
+  }, routeHandler)

src/routes/formsg/formsgSiteCreation.ts

@@ -11,6 +11,7 @@ import { BadRequestError } from "@errors/BadRequestError"
 import { getField } from "@utils/formsg-utils"
 
 import { attachFormSGHandler } from "@root/middleware"
+import { nameAnonymousMethods } from "@root/utils/apm-utils"
 import GitFileSystemService from "@services/db/GitFileSystemService"
 import UsersService from "@services/identity/UsersService"
 import InfraService from "@services/infra/InfraService"
@@ -45,6 +46,7 @@ export class FormsgSiteCreateRouter {
     this.infraService = infraService
     this.gitFileSystemService = gitFileSystemService
     // We need to bind all methods because we don't invoke them from the class directly
+    nameAnonymousMethods(this)
     autoBind(this)
   }
 

src/routes/formsg/formsgSiteLaunch.ts

@@ -27,6 +27,7 @@ import {
 import TRUSTED_AMPLIFY_CAA_RECORDS from "@root/types/caaAmplify"
 import { isErrnoException } from "@root/types/nodeError"
 import { SiteLaunchResult } from "@root/types/siteLaunch"
+import { nameAnonymousMethods } from "@root/utils/apm-utils"
 import UsersService from "@services/identity/UsersService"
 import InfraService from "@services/infra/InfraService"
 
@@ -150,6 +151,7 @@ export class FormsgSiteLaunchRouter {
     this.usersService = usersService
     this.infraService = infraService
     // We need to bind all methods because we don't invoke them from the class directly
+    nameAnonymousMethods(this)
     autoBind(this)
   }
 

src/routes/v2/authenticated/collaborators.ts

@@ -13,6 +13,7 @@ import { attachSiteHandler } from "@root/middleware"
 import { RouteCheckerMiddleware } from "@root/middleware/routeChecker"
 import { RequestHandler } from "@root/types"
 import { UserDto } from "@root/types/dto/review"
+import { nameAnonymousMethods } from "@root/utils/apm-utils"
 import { CreateCollaboratorRequestSchema } from "@root/validators/RequestSchema"
 import CollaboratorsService from "@services/identity/CollaboratorsService"
 
@@ -33,6 +34,7 @@ export class CollaboratorsRouter {
   }: CollaboratorsRouterProps) {
     this.collaboratorsService = collaboratorsService
     this.authorizationMiddleware = authorizationMiddleware
+    nameAnonymousMethods(this)
     autoBind(this)
   }
 

src/routes/v2/authenticated/notifications.ts

@@ -10,6 +10,7 @@ import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData"
 import { attachSiteHandler } from "@root/middleware"
 import { AuthorizationMiddleware } from "@root/middleware/authorization"
 import { RequestHandler } from "@root/types"
+import { nameAnonymousMethods } from "@root/utils/apm-utils"
 import NotificationsService, {
   NotificationResponse,
 } from "@services/identity/NotificationsService"
@@ -31,6 +32,7 @@ export class NotificationsRouter {
   }: NotificationsRouterProps) {
     this.notificationsService = notificationsService
     this.authorizationMiddleware = authorizationMiddleware
+    nameAnonymousMethods(this)
     autoBind(this)
   }
 

src/routes/v2/authenticated/review.ts

@@ -31,6 +31,7 @@ import {
   BlobDiffDto,
   EditedItemDto,
 } from "@root/types/dto/review"
+import { nameAnonymousMethods } from "@root/utils/apm-utils"
 import {
   CreateCommentSchema,
   CreateReviewRequestSchema,
@@ -66,6 +67,7 @@ export class ReviewsRouter {
     this.notificationsService = notificationsService
     this.githubService = githubService
 
+    nameAnonymousMethods(this)
     autoBind(this)
   }
 

src/routes/v2/authenticated/sites.ts

@@ -21,6 +21,7 @@ import { RepositoryData } from "@root/types/repoInfo"
 import { BrokenLinkErrorDto } from "@root/types/siteChecker"
 import { SiteInfo, SiteLaunchDto } from "@root/types/siteInfo"
 import { StagingBuildStatus } from "@root/types/stagingBuildStatus"
+import { nameAnonymousMethods } from "@root/utils/apm-utils"
 import {
   GetPreviewInfoSchema,
   LaunchSiteSchema,
@@ -60,6 +61,7 @@ export class SitesRouter {
     this.infraService = infraService
     this.repoCheckerService = repoCheckerService
     // We need to bind all methods because we don't invoke them from the class directly
+    nameAnonymousMethods(this)
     autoBind(this)
   }
 

src/routes/v2/authenticated/users.ts

@@ -13,6 +13,7 @@ import UserSessionData from "@classes/UserSessionData"
 
 import DatabaseError from "@root/errors/DatabaseError"
 import { isError, RequestHandler } from "@root/types"
+import { nameAnonymousMethods } from "@root/utils/apm-utils"
 import {
   VerifyEmailOtpSchema,
   VerifyMobileNumberOtpSchema,
@@ -29,6 +30,7 @@ export class UsersRouter {
 
   constructor({ usersService }: UsersRouterProps) {
     this.usersService = usersService
+    nameAnonymousMethods(this)
     autoBind(this)
   }
 

src/routes/v2/authenticatedSites/media.ts

@@ -17,6 +17,7 @@ import type {
   MediaFileOutput,
   RequestHandler,
 } from "@root/types"
+import { nameAnonymousMethods } from "@root/utils/apm-utils"
 import {
   CreateMediaDirectoryRequestSchema,
   CreateMediaFileRequestSchema,
@@ -44,6 +45,7 @@ export class MediaRouter {
     this.mediaFileService = mediaFileService
     this.mediaDirectoryService = mediaDirectoryService
     // We need to bind all methods because we don't invoke them from the class directly
+    nameAnonymousMethods(this)
     autoBind(this)
   }
 

src/routes/v2/authenticatedSites/repoManagement.ts

@@ -11,6 +11,7 @@ import UserWithSiteSessionData from "@root/classes/UserWithSiteSessionData"
 import GitFileSystemError from "@root/errors/GitFileSystemError"
 import { attachSiteHandler } from "@root/middleware"
 import type { RequestHandler } from "@root/types"
+import { nameAnonymousMethods } from "@root/utils/apm-utils"
 import { ResetRepoSchema } from "@root/validators/RequestSchema"
 import RepoManagementService from "@services/admin/RepoManagementService"
 
@@ -30,6 +31,7 @@ export class RepoManagementRouter {
   }: RepoManagementRouterProps) {
     this.repoManagementService = repoManagementService
     this.authorizationMiddleware = authorizationMiddleware
+    nameAnonymousMethods(this)
     autoBind(this)
   }
 

src/routes/v2/sgidAuth.ts

@@ -19,6 +19,7 @@ import {
 import { RequestHandler } from "@root/types"
 import { ResponseErrorBody } from "@root/types/dto/error"
 import { isPublicOfficerData, PublicOfficerData } from "@root/types/sgid"
+import { nameAnonymousMethods } from "@root/utils/apm-utils"
 import { isSecure } from "@root/utils/auth-utils"
 import { EmailSchema } from "@root/validators/RequestSchema"
 import UsersService from "@services/identity/UsersService"
@@ -42,6 +43,7 @@ export class SgidAuthRouter {
   constructor({ usersService, sgidAuthService }: SgidAuthRouterProps) {
     this.usersService = usersService
     this.sgidAuthService = sgidAuthService
+    nameAnonymousMethods(this)
     autoBind(this)
   }
 

src/utils/apm-utils.ts

@@ -0,0 +1,25 @@
+/* eslint-disable import/prefer-default-export */
+/**
+ * A function to set the name of anonymous methods on an object, all the way into the prototype chain
+ * This is useful to be reported in APM traces and spans
+ * This is an unconventional thing to do, so we have to disable a couple of eslint rules
+ */
+export const nameAnonymousMethods = <SelfType extends { [key: string]: any }>(
+  self: SelfType
+): SelfType => {
+  /* eslint-disable no-restricted-syntax */
+  /* eslint-disable no-continue */
+  for (const methodName in self) {
+    if (methodName === "constructor") continue
+
+    const method = self[methodName]
+    if (typeof method !== "function") continue
+    if (method.name) continue
+
+    Object.defineProperty(method, "name", {
+      value: methodName,
+      writable: false,
+    })
+  }
+  return self
+}