mod-dnstap: sink (file) is opened for truncate: should be documented

knot-2.5.4 with the following config:

mod-dnstap:
  - id: knot-2.5.4
    sink: /root/taps/knot-auth.tap

The file I reference for sink is opened for truncate: I believe this ought to be documented (possibly with a warning?) or rather changed to have the file opened for append. To be honest I was a little surprised that the file was truncated.

Related question: when knot is running, is there actually a signal I can send to it to have knot reopen the file? I'm thinking in terms of rotation.

Thanks for the observation. At least I will document it.

As for the log rotation, you can use the HUP signal which has the same effect as knotc reload. So the whole server has to be reloaded :-/. But wouldn't it be better to use a UNIX socket sink instead? In such a case you could have the dnstap output under your control.

@edmonds Please, is there any way how to enable appending for dnstap output files? I can't see it.

I agree with your observation of a UNIX socket, but since I was testing all I could, I thought I'd report that issue.

As to where the file is opened for writing, that piqued my curiosity (because BIND also does it, so I thought it's maybe not you :-)

I just now followed src/contrib/dnstap/writer.c to fstrm_file_writer_init(), and there, in fstrm-0.3.2/fstrm/file.c appears to be the "culprit" ;-)

fstrm_file_writer_init(const struct fstrm_file_options *fopt,
                       const struct fstrm_writer_options *wopt)
{
        struct fstrm_rdwr *rdwr = fstrm__file_init(fopt, 'w');
        if (!rdwr)
                return NULL;
        fstrm_rdwr_set_write(rdwr, fstrm__file_op_write);
        return fstrm_writer_init(wopt, &rdwr);
}

What I'd be interested to know, @edmonds, is why the file is opened for writing. Is that because it gets a header or something? That would explain the w. (Pardon my ignorance.)

@jpmens Yeah, the Frame Streams file format includes both a header and a footer. There is some documentation on the format buried in the libfstrm API documentation. The tl;dr is that I didn't consider the file appending use case much during the initial design because I was focused on the kinds of use cases where you have a lot of data coming in to busy nameservers and file rotation is mandatory. (Actually, I was probably even more focused on the socket transport rather than the file transport, because that lets you put all sorts of complicated processing rules or policies in the daemon consuming the data from the socket, similar to the syslog model.)

The way the format was designed makes it a bit complicated to implement an append mode now. First, we'd need to parse the header and verify that the Frame Streams content type matches what we want to write (i.e. protobuf:dnstap.Dnstap). If it doesn't match, should we refuse to start the daemon, start up without dnstap enabled, auto-rotate the output file, etc.?

Second, we'd need to verify that the file is appendable. Since the format is binary and payloads are length delimited, we'd need to verify that the end of the file isn't cut off in the middle of a payload. If we were to start appending payloads to such a file, it would corrupt the file, because a reader wouldn't identify the subsequently written length delimiters correctly. The end of a properly written Frame Streams file has a "STOP" frame at the end, which is the 12 octet sequence 00 00 00 00 (escape) + 00 00 00 04 (length) + 00 00 00 03 (control code "STOP"). Maybe you could get away with just checking the last 12 bytes of the file and rewinding the stream if they're present (maybe you get really unlucky and those 12 bytes are in the middle of a cut-off payload, though). The safest option is to just scan the entire file from the beginning to verify that the end of the file hasn't been cut off, but that's potentially a lot of I/O if the file is very large.

The format could have been made more complicated (e.g. when closing the file, backtrack to the beginning and write a field with the total length), but I also wanted to make it really easy to write an encoder and make it possible to generate a stream in one pass without having to rewind the stream.

Thank you very much, Robert, for the explanation. And please: perish the thought of making things more complex.

@edmonds Thanks a lot for the comment!

@jpmens So do you agree with just extending the documentation as a resolution of this issue?

@dsalzman absolutely. Please close when suitable.

That all being said, overwriting a previously written dnstap file is kind of user unfriendly behavior, so maybe there should be some safe guards to avoid that.

One strategy I kind of like is to use a temporary file name while writing the file, and once the file has been finished and closed, rename it to the final name. E.g., maybe you initially start writing to /root/taps/.knot-auth.tap.tmp-iYiec2ju, then on closing the file it gets renamed to the real filename /root/taps/knot-auth.tap. Or maybe the final filename isn't fixed and is instead generated from a template that's passed through strftime() so you have a series of files with timestamps built into the name.

Another advantage of the initial temporary name scheme is you can have a post-processor that consumes e.g. /root/taps/*.tap, and this scheme ensures that the post-processor never opens a file while the server is still writing it.

But, maybe this is a bunch of complexity that should really be in a dedicated utility rather than in the nameserver, and that's why the Frame Streams socket transport exists.

mentioned in merge request !827 (merged)

closed via commit a97fc24e

closed via merge request !827 (merged)

mentioned in commit a97fc24e