Skip to content

Conversation

@Joe-Edwards-GDS
Copy link
Contributor

Proposed changes

What changed

  • Commonised additional log properties
  • Added Dynatrace trace id via the DT SDK

Why did it change

  • So we can connect logs with DT traces

@Joe-Edwards-GDS Joe-Edwards-GDS requested review from a team as code owners May 19, 2025 10:05
sessionId: req.session?.id,
});
next();
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure where this came from:

  • We seem to do this twice - once on everything (above) and once on the router only, but it's identical
  • The pino-http middleware already sets req.log for us

If there's a good reason then happy to keep it...

Copy link
Contributor

Choose a reason for hiding this comment

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

Git says you're the one who added both those blocks... so I think it's up to you to say whether there's a reason.

Is there a reason this PR is still a draft?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I suspect they appeared via a merge conflict but I'm not sure - happy to say we only need one, and perhaps that should be extracted to a separate PR.

This specific PR is still in draft because we've been directed not to tie ourselves into Dynatrace-specific libraries. Ideally we'd use OpenTelemetry instead (as an open standard) but I couldn't get that to work.

Fine to close this if you're not missing the functionality!

@sonarqubecloud
Copy link

@Joe-Edwards-GDS Joe-Edwards-GDS marked this pull request as draft May 20, 2025 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants