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

Turn off need to "registerCustomFields" #46

Closed 0x203 closed 4 years ago

0x203 commented 4 years ago

First of all: thanks for that library! We find it useful and are happy to have it.

Context

We try to use structured logging wherever possible and favour that al lot over string interpolation for the log messages. This allows us to search and analyse logs far more easy. I.e.

// better
logger.info("Thingy created", { user: userId, thingy: { foo, bar }});
// than
logger.info(`User ${userId} created a Thingy with foo "${foo}" and bar "${bar}`);

So, we've made extensive use of those custom_fileds with your library in version 5.3.2. We've even written a small wrapper to make this simpler for us.

Thus, we were very pleased with seeing that version 6.0.0 introduced basically exactly what we've used our wrapper for: child loggers and better support for custom fields.

Problem

Since we make heavy use of these custom fields, for us it's a huge overhead to register them all with registerCustomFields first. We could do this in our wrapper dynamically, but this would be a ugly workaround.

Request

We would to be able to turn off the requirement to register all possible custom fields in order for them to be logged. We don't care how exactly this would happen, as long as it's a one time action (potentially per logger).

We could imagine this could be done either globally (requireCustomFieldRegistration = false) or per logger (logger.setRequireCustomFieldRegistration(false)).

Are you fine with this?

If yes, we would be happy. We could open a PR if you like. If no, we would rather stay with version 5, because there everything still works for us. However, it would be sad if we miss other features in newer versions then.

KarstenSchnitter commented 4 years ago

Thanks, for you nice feedback. Let me explain a little bit about the need to register custom fields. You are always welcome to add any fields you like to the structure of the log message. However, declaring a field as a custom field is a feature specifically targeting the SAP Cloud Platform. It tells the Application Logging Service to parse this field (as a string) and provide it as a searchable field to Kibana. This is a decision, that should be taken deliberately, considering the need for each field, since the overall number of custom fields is limited. The registration mechanism in its current form was introduced, to avoid the accidental creation of custom fields. If you create too many custom fields you may find your app blocked from the Application Logging Service. Note, that if you do not register your fields as custom fields, they will still be part of the message in Kibana as top-level fields of the JSON message. The only drawback is, that they are not indexed, so that you cannot build visualisations based on those fields.

With that in mind, I would conclude, that the custom fields mechanism is a very specific feature of this library with respect to the SAP Cloud Platform. It will only be useful to applications, running on that platform. As developers of the Application Logging Service we need to protect our stack against the ingestion of too many custom fields. If we drop the registration in the library, we would need to employ other measures, e.g. registration of custom fields during the bind service operation. But this would mean an even worse experience. If you have other suggestions, please let us know.

christiand93 commented 4 years ago

Please note that unregistered custom fields will not show up in logging messages at all currently. However, we will add a mechanism to log any unregistered custom fields as top-level fields to make them available as non-indexed fields in Kibana.

christiand93 commented 4 years ago

As of version 6.1.0 unregistered fields will be added as top-level fields.