-
Notifications
You must be signed in to change notification settings - Fork 0
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
Structured JSON Logging #14
Conversation
…sh` and all jobs use `nix-shell --run`
👇 Click on the image for a new way to code review
Legend |
So the issue is that log data is actually allowed to be an arbitrary object, whereas here the It converts them to strings first. Structured logging benefits from potentially arbitrary keys, and to allow nested structure, not just strings. This means, the |
Ok let's not do However as for the schema, the GELF can work: https://docs.graylog.org/docs/gelf#gelf-payload-specification. Not sure about other ones. Now the only thing left is the fact that data structures should be usable with The With this concept, it's also possible to build a context for further logging. And that just means a prefilled dictionary. Somewhat similar to creating child loggers as we do right now. https://github.com/bitc/structured-logging-schema#message-and-payload So it does seem all we need to do is to enable the usage of a payload structure.
Then we have a default structure associated to the log... think of it as "global" context, like time and such. But then further context can be created for a subset of loggers... And "canonical logging" means that the log gets built up and then output at the end. However that doesn't cover a span. You want to know when it started and stopped too. It's possible that we combine the structure with the message too, but that might a bit messy. It really depends on whether a log always requires a message or if it can be optional. For example:
Then a default key can be mandated. |
Another example https://www.elastic.co/guide/en/ecs/current/ecs-reference.html. In this case, it does seem that special casing a message is the wrong way to go, if an object is used... you just need to specify your own schema like |
Won't build in any special schemas. This can be dictated custom by users. PK application can choose to use something custom. Both for formatter and the structured data. Contexts for spans and traces can be done through having a prefixed object context. This can also be added to a child logger during construction. |
Well this was quite easy. All we needed to do is to add:
Then when constructing logger:
And that's it, you get JSON records being output now. |
The main issue now is schema standardisation. There's no real one single standard for logging, common ones include graylog GELF, the ECS fields above. Libraries like structlog don't really specify any standards for the payload. OpenTelemetry does have a standard for their properties. There are some "base"/core properties that these libraries tend to all use:
We have these pieces of information as well in the To set a base-schema, we can use follow ECS's core properties: https://www.elastic.co/guide/en/ecs/current/ecs-guidelines.html https://www.elastic.co/guide/en/ecs/current/ecs-base.html https://github.com/elastic/ecs/blob/main/generated/csv/fields.csv These are the base fields for ECS:
The log level is placed in a subobject: https://www.elastic.co/guide/en/ecs/current/ecs-log.html, intended to be a keyword, not a number. |
Hmm to make this elegant, we need to do some changes:
|
It appears open telemetry considers logging out to stderr as the telemetry/trace exporter to be used only for "debugging". Production exporters all use TCP, GRPC or HTTP 1.1 to send traces to a centralised trace collector like zipkin or whatever. However this seems to be against how traditional orchestration is supposed to work, where this data goes to STDERR, and then collected by the orchestrator to be sent to centralised loggers. Why did the open telemetry people decide to create their own exporters? Was it to get around STDERR, was it because STDERR logging is considered orthogonal?
It's all done via http or grpc. As if networked sends is better than stderr. Maybe interleaving with stderr isn't right. However for decentralised systems, does it make sense to assume that there is a collector somewhere else? Obviously we would say that the CLI/GUI is a viable collector in order to debug the system. Whereas our hosted testnet may have be pointed somewhere else. It does seem that stderr/stdout is being deprecated for more "network-centric" observability. I guess it provides more flexibility. |
Otel is way too complex. Here's apparently the "basic" example of using Otel and exporting directly to the console: https://github.com/open-telemetry/opentelemetry-js/blob/main/examples/basic-tracer-node/index.js. Why does it require so many different packages?
It seems over-engineered. The spec however might be useful. |
So in-process sending is only for adoption as I suspected. However the community has obviously selected against relying on STDERR, because they view traces not as human readable information, but as information intended for machines to parse, and thus did not bother writing to STDERR, except as an afterthought. This makes sense, because for them once the span-data is written to stderr what happens to it afterwards? It's sort of useless as textual information (not really but one can understand). So there's nothing built to "forward" the stderr logs to a trace receiver or OTLP collector. So even with the OTLP collector it's all designed to send logs over a network connection rather than relying on simple STDERR. I think what I'll do is examine the spec of opentracing with respect to the log data itself, and then just add it to our existing structured logging, which continues to be send to STDERR. For future sending to other places, that could be done via custom handlers. For viewing and debugging the traces, if we maintain compatibility with the spec, it should be possible to send the stderr logs directly into a trace collector via HTTP 1.1: https://opentelemetry.io/docs/concepts/sdk-configuration/otlp-exporter-configuration/. Also see https://github.com/opentracing-contrib/java-span-reporter |
An example of manually sending a trace: Get a Zipkin payload to test. For example create a file called trace.json that contains:
With the Collector running, send this payload to the Collector. For example:
|
Opentelemetry details are moved into new issue #15. Because the telemetry schema doesn't actually require a mandatory message, it makes sense to change our logging methods to take a string or object. If string, then that's assumed to be a regular It's up to users to provide the necessary message with I imagine tracing would be built on top of js-logger, so this library is kept simple. Tracing would be PK-specific. |
1dd9eac
to
879cb4f
Compare
…ng for bash scripts
29fe85a
to
66f9e5d
Compare
This is now done. Going to update the |
Description
This introduces structured JSON logging. As an intermediate step towards tracing and machine parseable data that can be used to construct contexts and traces.
We already have a
LogFormatter
type. Any thing that produces a string given aLogRecord
is sufficient.Usually we construct formatters with the
formatting.format
function, which takes template strings and produces a string.We can provide something like:
That produces a formatter function. The
json
symbol simply means the entire data structure of theLogRecord
is captured.The other issue is the schema of the keys of the JSON record, we should align it with some standard. Possibly close to opentracing standard or structlog standard.
Issues Fixed
Tasks
jsonFormatter
as a default JSONLogFormatter
trace
tostack
to avoid confusion with tracing in Integrate Tracing (derived from OpenTelemetry) #15stack
andkeys
to be lazily created if the renderer calls themLogData
for arbitrary data to fit into the logLogData
enables lazy values by wrapping values in a lambdajest.useRealTimers();
to close off the timer mocking in testsmsg
is no longer mandatory, if is not passed, themsg
will be emptyFinal checklist