Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MTTL-376 Setup boiler plate code for logging the correlation ID #2

Merged
merged 7 commits into from
Mar 18, 2021

Conversation

evak6979
Copy link
Contributor

@evak6979 evak6979 commented Mar 15, 2021

Link to JIRA ticket

https://hackney.atlassian.net/browse/MTTL-376

Describe this PR

PR to add correlationId for every incoming request.

What is the problem we're trying to solve

CorrelationId will be used as part of our logging strategy. By including a CorrelationId with logs we will be able to trace individual requests during debugging/monitoring attempts.

What changes have we introduced

  • Added a correlationId Middleware
  • Initialized it in startup
  • Added a constants file
  • Added unit tests

Checklist

  • Added end-to-end (i.e. integration) tests where necessary e.g to test complete functionality of a newly added feature
  • Added tests to cover all new production code
  • Added comments to the README or updated relevant documentation (add link to documentation), where necessary.
  • Checked all code for possible refactoring
  • Code pipeline builds correctly

Follow up actions after merging PR

Are there any next steps that need to addressed after merging this PR? Add them here.

@evak6979 evak6979 changed the title Mttl 376 correlation MTTL-376 Setup boiler plate code for logging the correlation ID Mar 15, 2021
@evak6979 evak6979 marked this pull request as ready for review March 15, 2021 12:53
using Microsoft.AspNetCore.Routing;
using NUnit.Framework;

namespace BaseApi.Tests.V1.Controllers
Copy link
Contributor

Choose a reason for hiding this comment

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

The folder names need to be changed (both BaseAPI and BaseAPI.Tests)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Happy to do so, but if possible we should merge the initial renaming PR. And then I can merge master from there to this branch which should resolve it.

Copy link
Contributor

Choose a reason for hiding this comment

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

I forgot the renaming PR hasn't been merged, ignore this comment please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No worries at all :) With 2 approvals, I took the liberty of merging the renaming PR to master and then into this branch. Renaming issues should be resolved by now.

@evak6979 evak6979 merged commit faa45fc into master Mar 18, 2021
@evak6979 evak6979 deleted the MTTL-376-CorrelationId branch March 18, 2021 10:13
AnnaGolosova added a commit that referenced this pull request Jan 21, 2022
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