SAP / cf-nodejs-logging-support

Node.js Logging Support for Cloud Foundry provides the creation of structured log messages and the collection of request metrics
https://sap.github.io/cf-nodejs-logging-support/
Apache License 2.0
43 stars 22 forks source link

Express: Set tenant id after logNetwork call #51

Closed Magoli1 closed 4 years ago

Magoli1 commented 4 years ago

Hey everyone,

when working with this library together with the xs-security library from SAP, I encounter the following problem:

I have two routers: One for unauthenticated calls to my app, one for authenticated (JWT) ones. As per my understanding, when the request arrives at the app, the logNetwork function gets executed and due to this, also the tenant_id is set. If there is no tenant id in the header field present, "-" will be set as default. This behavior is fine for the unauthenticated routes. But for the authenticated ones, I would like to be able to add the tenant id, after the call passed the passport strategy (and the subaccount id was extracted from the token), so that I have reference to the right tenant.

I have a, in my eyes, rather hacky solution, which is putting another middleware before the logNetwork call, where the authorization header is checked, and if a JWT is present, the tenant id will be extracted. But doing it this way, we have basically two places where I need to check for the authorization header. I know that there are custom fields, but I would like to set the root field, to not have multiple possible fields afterwards.

I would like to see some function like the setCorrelationId, also for the tenant (maybe for all root fields?). Is there already a way to achieve this? Let me know your thoughts 😀

BR Robert

Magoli1 commented 4 years ago

Hey,

Is there any solution to solve this issue?

BR Robert

nicklas-dohrn commented 4 years ago

Hey Robert, Sorry for the late response. As you might know, in nodejs because of its javascript roots, you can rewrite every value of every object you have at hand. the logging context of every request is stored in the req object, therefore you can access (and therefore set) the tenant_id field via: req.logger.logObject.tenant_id. please note that you can cause parsing issues if your changes do not comply with the type of the field you are changing and that the tenant_id has the correct format. This is also the reason why we refrained from implementing setter functions for every field besides the correlation_id. This change to the tenant_id can be at any point during the handling of a request, because the log for any request contains fields, which can only be calculated when the response will be sent.

We will take a look at the pull request and decide If we want to change in this direction. Greetings, Nicklas

Magoli1 commented 4 years ago

Hey Nicklas,

thanks for your response! Yes I did just change it in the way you recommended, but I thought that it might be a better way to have a setter and a getter for that one to ensure that everything internally works in the way it should (thats even more true for prod apps) 👍

Thank you!

BR Robert

Magoli1 commented 4 years ago

Any plans on publishing the changes to NPM?

BR Robert

christiand93 commented 4 years ago

Hi Robert,

we have released version 6.2.0, which is now available via NPM.

Kind regards, Christian