SAP / cloud-sdk-js

Use the SAP Cloud SDK for JavaScript / TypeScript to reduce development effort when building applications on SAP Business Technology Platform that communicate with SAP solutions and services such as SAP S/4HANA Cloud, SAP SuccessFactors, and many others.
Apache License 2.0
166 stars 57 forks source link

Requirements for logging in a multi-tenancy environment #484

Open jwe-cdn opened 4 years ago

jwe-cdn commented 4 years ago

Hello SAP Cloud SDK team,

Is your feature request related to a problem? Please describe. In a multi-tenancy environment, it is important for us to recognize where the logging outputs are coming from.

Describe the solution you'd like We need the ability to set Kibana metadata fields (correlation_id, tenant_id, tenant_subdomain, remote_user, #cf (Custom Fields), ...) for the logging outputs on the various functionalities/classes within the SAP Cloud SDK. For example, it should be possible to set these Kibana metadata fields individually for each request (RequestBuilder).

Best regards Jens

artemkovalyov commented 4 years ago

Hi @jwe-cdn,

Thanks for the idea and suggestion. I'll add it to the backlog. Do you have an idea of an API for this feature that would work for you? If you have a solution in mind we'd be happy to review a pull request.

How urgent is this for your project? Do you have a workaround to circumvent this limitation at the moment?

Best, Artem

artemkovalyov commented 4 years ago

Taking the opportunity I'd also be happy if you can take our survey to help us deliver a better product: https://sapinsights.eu.qualtrics.com/jfe/form/SV_0pUmWpCadpoLhyZ

jwe-cdn commented 4 years ago

Hello SAP Cloud SDK team,

the priority of this feature request is low. We are currently working on creating log entries that clearly show where they come from (Which request, tenant, ...?). We discovered that the log entries generated by the SAP Cloud SDK are missing something like correlationId or tenantId. We don't have a workaround for this. That's why we created this feature request. Our self-generated log entries contain these necessary metadata fields and can therefore be clearly evaluated in Kibana. Unfortunately, we don't have a direct solution idea for this problem in the SAP Cloud SDK. The requirement is probably not easy to implement, because these metadata are required at every point in the SAP Cloud SDK where log entries are generated.

Best regards Jens

artemkovalyov commented 4 years ago

Hi @jwe-cdn,

Thanks for sharing the status and your prios. We have it in the top part of our backlog and will be updating you when it's done or we have additional questions in when developing this logging improvement.

Best, Artem

artemkovalyov commented 4 years ago

Hi @jwe-cdn,

What Node version do you use productively?

Best, Artem

jwe-cdn commented 4 years ago

Hello SAP Cloud SDK team,

we are currently using version 10 in the latest version. Currently this is version 10.23.0.

Best regards Jens

FrankEssenberger commented 4 years ago

hm, we currently plan to use continued local storage to have the request context in the call stack. Unfortunately this functionality is not available in node 10. See our evaluation here.

jwe-cdn commented 4 years ago

Hello SAP Cloud SDK team,

we will check if we can upgrade to node version 13 or higher. We had already tried to switch to node version 12, but there were some problems. But it was a long time ago.

Best regards Jens

jwe-cdn commented 4 years ago

Hello SAP Cloud SDK team,

a first quick test showed us the following problems. at the moment it is not possible to switch to node version 13 or higher for these reasons:

SAP NPM package "@sap/hdbext" (we using Version 7.0.0) https://www.npmjs.com/package/@sap/hdbext "engines": { "node": "^8.0.0 || ^10.0.0 || ^12.0.0" }

Angular NPM package "@angular/cli" (we using 10.2.0) https://www.npmjs.com/package/@angular/cli https://github.com/angular/angular-cli/blob/master/package.json "engines": { "node": ">=10.13.0 <13.0.0" }

Both packages are essential for us!

Best regards Jens

FrankEssenberger commented 4 years ago

I asked the colleagues from @sap/hdbext if they could also support higher node versions. Also note that the performance is not so great for node 12 but it will workd. Regarding the angular cli I wonder if this is not a dev dependency and you can ignore it?

jwe-cdn commented 4 years ago

You are right "@angular/cli" is a dev dependency. We are currently checking whether we get any problems here when developing locally with node.js version 13 or higher.

marikaner commented 4 years ago

@jwe-cdn, please note that this repository has been moved and the link to this issue has changed.

FrankEssenberger commented 4 years ago

I heard back from the hdbext colleagues and in December an update on the hana-client will take place supporting node 14. This is their last blocker. After this they will also offer node 14 support.

jwe-cdn commented 4 years ago

After our last tests we would now say that if we get a Node.js 14 compatible npm package "@sap/hdbext" nothing should speak against switching to Node.js 14. We have roughly tested everything with Node.js 14 (v14.15.0) and there are only problems/errors with the npm install of the "@sap/hdbext" package.

FATAL ERROR: A fatal error occurred during install. Node version v14.15.0 is unsupported by @sap/hana-client. Current supported versions are 8, 10, and 12 npm WARN notsup Unsupported engine for @sap/hdbext@7.0.1: wanted: {"node":"^8.0.0 || ^10.0.0 || ^12.0.0"} (current: {"node":"14.15.0","npm":"6.14.8"}) npm WARN notsup Not compatible with your version of node/npm: @sap/hdbext@7.0.1

There are some other packages that currently have a lower Node.js version than 14 specified in the package.json under engines/node, but it seems that there are no problems here.

jwe-cdn commented 3 years ago

Here again a short feedback on the requested Node.js 14 usage... The SAP package @sap/hdbext is now fully Node.js 14 compatible. However, there are still SAP packages that generate NPM warnings during NPM install regarding potentially non-guaranteed compatibility with Node.js 14. This is mainly the SAP package @sap/approuter but also a dependency in the SAP Cloud SDK for JavaScript generator package @sap-cloud-sdk/generator. The NPM warnings in the SAP package @sap/approuter mainly relate to dependencies on SAP packages of older versions, for which there are newer major versions that are already compatible with Node.js 14. The use of the latest SAP packages is usually simply missing here. However, we do not have a point of contact for the SAP package @sap/approuter. With the SAP Cloud SDK for JavaScript there is only one NPM warning in the generator @sap-cloud-sdk/generator. Here are the corresponding NPM warnings in the corresponding NPM packages:

@sap/approuter npm WARN notsup Unsupported engine for @sap/approuter@9.1.0: wanted: {"node":"^10.0.0 || ^12.0.0"} (current: {"node":"14.15.5","npm":"6.14.11"}) npm WARN notsup Not compatible with your version of node/npm: @sap/approuter@9.1.0 npm WARN notsup Unsupported engine for @sap/xsenv@2.2.0: wanted: {"node":"^6.0.0 || ^8.0.0 || ^10.0.0 || ^12.0.0"} (current: {"node":"14.15.5","npm":"6.14.11"}) npm WARN notsup Not compatible with your version of node/npm: @sap/xsenv@2.2.0 npm WARN notsup Unsupported engine for @sap/audit-logging@3.2.0: wanted: {"node":"^6.0.0 || ^8.0.0 || ^10.0.0 || ^12.0.0"} (current: {"node":"14.15.5","npm":"6.14.11"}) npm WARN notsup Not compatible with your version of node/npm: @sap/audit-logging@3.2.0 npm WARN notsup Unsupported engine for @sap/e2e-trace@2.1.0: wanted: {"node":"^6.0.0 || ^8.0.0 || ^10.0.0 || ^12.0.0"} (current: {"node":"14.15.5","npm":"6.14.11"}) npm WARN notsup Not compatible with your version of node/npm: @sap/e2e-trace@2.1.0

@sap-cloud-sdk/generator npm WARN notsup Unsupported engine for @sap/edm-converters@1.0.40: wanted: {"node":"10.x.x || 12.x.x"} (current: {"node":"14.15.5","npm":"6.14.11"}) npm WARN notsup Not compatible with your version of node/npm: @sap/edm-converters@1.0.40

jjtang1985 commented 3 years ago

Hi @jwe-cdn , I found the support channel of the @sap/approuter dependency from npm: https://www.npmjs.com/package/@sap/approuter#getting-support

For the @sap/edm-converters dependency used by the @sap-cloud-sdk/generator, I will contact them and let you know.

@artemkovalyov , FYI, the node 14 request.

jjtang1985 commented 3 years ago

I created this issue for the node 14 request, as this is a complete different topic.

jwe-cdn commented 3 years ago

Just to clarify... Your colleagues asked us if we had a problem when this requested feature be implemented with Node.js 14 (See above!). My last feedback answer related to that too.