Refactor CSV Logging

[mTsBucy1]

To fix #515 I added a macro that properly escapes the values.
I like this approach because it is very lightweight and doesn't add any csv reader functionality as many external dependencies would. On the other hand, this still struggles with nested data and Option<> as well as hard coded headers.

However, currently in order to use the macro in the tests, I use macro_export, but then this leaks into the docs. Does anyone know how to prevent this in Edition 2018? Or is this a case for docs::hidden?

I additionally propose to flatten the GooseRequestMetric.raw into its fields method, url, headers, body for the request log and error log. For GooseDebug, which includes a whole GooseRequestMetric, a similar argument applies.
The Option<> values should further be represented by an empty string when None.
Since those changes potentially break external metric analysis via CSV import, I am reluctant to make these changes without further input.
What do you think? Do you import goose metrics in another program? And via csv or other?

[jeremyandrews]

I like this approach because it is very lightweight and doesn't add any csv reader functionality as many external dependencies would. On the other hand, this still struggles with nested data and Option<> as well as hard coded headers.

In my local testing, it fixes the bug you reported and I don't see any regressions. Where are you running into problems with nested data and Option<>?

However, currently in order to use the macro in the tests, I use macro_export, but then this leaks into the docs. Does anyone know how to prevent this in Edition 2018? Or is this a case for docs::hidden?

Doing some searches, I think this is indeed the correct solution (and confirmed locally it works as expected):
https://doc.rust-lang.org/rustdoc/write-documentation/the-doc-attribute.html#hidden

I additionally propose to flatten the GooseRequestMetric.raw into its fields method, url, headers, body for the request log and error log. For GooseDebug, which includes a whole GooseRequestMetric, a similar argument applies.

I'm not sure I follow what you're suggesting. Are you proposing replacing the "raw" column with a column for each of the 4 contained fields? I don't see any fundamental problem with doing that.

Since those changes potentially break external metric analysis via CSV import, I am reluctant to make these changes without further input.

I'd prefer to get this fix in first, before making further changes. Changing the format of the CSV export would require a version bump (ie going to 0.17).

Pleae do add an entry to the Changelog.

What do you think? Do you import goose metrics in another program? And via csv or other?

I use the json format, and do processing of fields on the command line with jq (see this example for example).

[mTsBucy1]

I like this approach because it is very lightweight and doesn't add any csv reader functionality as many external dependencies would. On the other hand, this still struggles with nested data and Option<> as well as hard coded headers.

In my local testing, it fixes the bug you reported and I don't see any regressions. Where are you running into problems with nested data and Option<>?

There is no problem per se, but the existing TODO hasn't been addressed. Specifically, the csv still contains "Some(value)" and "None", instead of the more CSV-style empty string and "value".

I additionally propose to flatten the GooseRequestMetric.raw into its fields method, url, headers, body for the request log and error log. For GooseDebug, which includes a whole GooseRequestMetric, a similar argument applies.

I'm not sure I follow what you're suggesting. Are you proposing replacing the "raw" column with a column for each of the 4 contained fields? I don't see any fundamental problem with doing that.

Yes, that is exactly what I meant. It feels wrong to have json-like fields in a csv file. But that is material for a future PR.

[jeremyandrews]

Thanks, this is a much appreciated improvement: confirmed that CSV import works correctly now. Waiting for tests to pass then will merge.

The suggested followups would be very welcome if you wish to submit additional PRs, both expanding GooseRawRequest to proper columns, and properly formatting Option<>. ///