-
Notifications
You must be signed in to change notification settings - Fork 14
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
upstream of transparanty kms logger #195
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@beejones Did you check how the actual CCF log looks like with B&A services used? If not can you do that please? If it prints out too much warnings maybe we want to consider not putting them as warnings to avoid people from ignoring other important warnings.
Logger.debug( | ||
`isValidJwtToken: ${key}, expected: ${policy[key]}, found: ${jwtProp}, ${compliant}`, | ||
Logger.info( | ||
`isValidJwtToken: ${key}, expected: ${policy[key]}, found: ${jwtProp}, ${compliant}`, logContext |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's our criteria for using debug and using info?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DEBUG is only for debugging. There are too many Logger.info calls because debug was broken. I fixed this in the new PR:
#221
There is minimal info logging for the endpoints. In principal just the name of request and the result of the endpoint. With debug flag you see all details
CI with better CCF log: https://github.com/microsoft/privacy-sandbox-dev/actions/runs/11834527642 |
Ronny triggered new CI: CI · microsoft/privacy-sandbox-dev@9dc0c53. Thanks! Looks like printing out some objects is not successful e.g. |
We need to sync the codebase of https://github.com/microsoft/azure-privacy-sandbox-kms/ with https://github.com/microsoft/azure-transparent-kms because this repo has important changes which are beneficial for azure-transparent-kms. The PR was needed to upstream big changes in azure-transparent-kms to this repo. Once this PR is merged, it will be possible to merge the changes into azure-transparent-kms with minimal merge conflicts. |
This is fixed in #221 |
Extend Logger with LogContext including scope and request id. This allow us to correlate requests with the logs in KMS in order to debug transactions.
The basis of this development was done by @yf23.
Request ID enhancement
uuid
package working. Preferrable to generate random UUID.x-ms-kms-request-id
Log Enrichment with
LogContext
Add function scope stack and request ID to KMS log with
LogContext
. The output format is[requestId=***,scope=***]