foundationdb

foundationdb copied to clipboard

This PR supports tracing non-string value fields in JSON tracing.

Changes in this PR:

• Fix a bug in the flowbench driver
• Add microbenchmarking for generating trace events
• Various minor tracing performance optimizations to avoid unnecessary copies
• Add a TraceValue class to help the JsonTraceLogFormatter differentiate between string value fields and all other fields.
• Support customized tracing for booleans, lists, numbers, counters, and strings

Style

• [x] All variable and function names make sense.
• [x] The code is properly formatted (consider running git clang-format).

Performance

• [x] All CPU-hot paths are well optimized.
• [x] The proper containers are used (for example std::vector vs VectorRef).
• [x] There are no new known SlowTask traces.

Testing

• [ ] The code was sufficiently tested in simulation.
• [x] If there are new parameters or knobs, different values are tested in simulation.
• [x] ~ASSERT, ASSERT_WE_THINK, and TEST macros are added in appropriate places.~
• [x] Unit tests were added for new algorithms and data structure that make sense to unit-test
• [x] ~If this is a bugfix: there is a test that can easily reproduce the bug.~

@fdb-build test this please

I haven't carefully reviewed the code, but I just wanted to leave one overall thought here.

My preference for how we would do this would be that we would not convert the objects to strings in the JSON-style, but rather allow the formatter in use to apply a desired formatting to them. That could potentially be done by either using the formatter to apply a style at log time or by storing typed values and converting them at write time. One downside of the former is that it wouldn't work as well if we ever support multiple simultaneous log formatters [1], but maybe that's ok.

Doing it the way it's done here seems like it would work well in the case that we rely only on the formatters we have now, and it's probably not the end of the world to close the door a little bit on other options. All else being equal, though, it would be a little sad if we undid the work to make the trace logging framework more general.

[1] This is something I actually would like to support one day. My motivating use-case is that I'd like the ability to generate logs that get simultaneously written to both a file and to stdout, and I would like to use a different format for the stdout version.

My preference for how we would do this would be that we would not convert the objects to strings in the JSON-style, but rather allow the formatter in use to apply a desired formatting to them.

That is the current design, the formatting of TraceValue objects is handled by the ITraceLogFormatter class, so XML tracing does not change at all and other ITraceLogFormatter implementations can be added.

That is the current design

Right you are, I somehow missed this change. Sorry for the noise.

One option that we already talked about some through other channels is to make this formatter a differently named, distinct option from the existing json formatter to aid in migrating between the two.

Having two options would allow time to migrate any tooling that uses the existing logs, and it also makes it easier to minimize the window when you would need to support both log formats (with one supported format, this is controlled strictly by upgrades that tend to be spread out in time).

AWS CodeBuild CI Report

• CodeBuild project: foundationdb-pull-request-build
• Commit ID: 2c759e5e4d855d40bef954c9e39665380983f806
• Result: FAILED
• Build Logs (available for 7 days)