access log: add JSON logging mode #4693

aa-stripe · 2018-10-11T20:36:28Z

These are the code changes to add a JSON access logging mode. I am working on the docs/release notes, but wanted to get started on code review!

Description:

Add a new config option under access_log called json_format. This is a single level dictionary that contains strings as keys, and envoy access log format specifiers (such as %PROTOCOL%) as values. The specifiers will be replaced with actual values at logging time. I call this dictionary the "format dictionary" (as opposed to "format string").
You can specify only one of format (format string) or json_format (format dictionary). If neither are there, we fall back to the default string format.
Add the correct plumbing inside the configuration parsing to handle this.
Add a new access log formatter class that is instantiated with the format dictionary. It maintains the mapping of dictionary keys to loggers
Create a new class called FormatterProvider, to distinguish things that actually extract the information from a request. The things that combine together a bunch of FormatterProviders are still called Formatters. This is primarily a semantic/naming difference, but imo these are two conceptually separate things. There is, however no API difference, and if people are truly opposed to this, I could just merge them back into one Formatter class. This also provides a better foundation for adding more log formats in the future.
At present, only one specifier per key in the format dictionary is allowed. This is because the whole point of JSON logging is to make logs easily machine-parseable. If you can include multiple formats in the same field, then you'll be right back to parsing those manually
At present, only top-level keys are allowed in the format dictionary. This is validated at config load time. In the future, we can expand this to have nested dictionaries.

Risk Level:
Low. It's an optional feature that has to be explicitly enabled.

Testing:
Unit testing for the actual formatter, and config load. Also manually tested using an example config file.

Docs:

Amended Access Log docs to create a notion of "Format Strings" and "Format Dictionaries".
- Put things that are common to access logs in general under "Format Rules", and then distinguished how strings and dictionaries are different.
- Called out restrictions on format dictionaries
Added protobuf comments for format and json_format

Release Notes:
Will have to add Release notes

Fixes #2692

Signed-off-by: Aaltan Ahmad <[email protected]>

…om google::protobuf to Protobuf Signed-off-by: Aaltan Ahmad <[email protected]>

Signed-off-by: Aaltan Ahmad <[email protected]>

aa-stripe · 2018-10-11T21:14:50Z

@alyssawilk This should work now. I think doing the rebase/signoff thing after rebasing on master confused things a bit.

cc'ing @mattklein123 @htuch @ggreenway

aa-stripe · 2018-10-11T21:15:27Z

I also noticed the Once you submit a PR, please do not rebase it guidance

htuch

Thanks for the contribution! Some comments to get started.

htuch · 2018-10-11T22:07:24Z

api/envoy/config/accesslog/v2/file.proto

-  string format = 2;
+  oneof access_log_format {
+    string format = 2;
+    google.protobuf.Struct json_format = 3;


Can you add a comment describing how this is interpreted? E.g. can we use the same custom access log formats in leaf strings, limitations on depth of nesting, etc.

Yup, this will be coming soon with the docs updates.

protobuf comment

htuch · 2018-10-11T22:09:04Z

include/envoy/access_log/access_log.h

+public:
+  virtual ~FormatterProvider() {}
+
+  virtual std::string format(const Http::HeaderMap& request_headers,


Can you add a Doxygen comment here with args/return explanation?

Agreed

doxygen comment on FormatterProvider

Felt like it was redundant to call out the args, so just added an explanation and then an @return

Envoy include/envoy APIs do have a verbose/redundant format, the best way to think of them is as official documentation that could ship without even the code (even though nobody uses them that way today, this is the style..).

htuch · 2018-10-11T22:11:02Z

include/envoy/access_log/access_log.h

@@ -84,5 +84,20 @@ class Formatter {

 typedef std::unique_ptr<Formatter> FormatterPtr;

+/**
+ * Interface for access log provider


Can you put this explanation that you have of Formatter vs FormatterProvider in the PR commit message here as well?

Agreed

Add FormatterProvider explanation

htuch · 2018-10-11T22:12:01Z

source/common/access_log/access_log_formatter.cc

+  for (const auto& pair : format_mapping) {
+    auto providers = AccessLogFormatParser::parse(pair.second);
+
+    // Enforce that each key only has one format specifier in it


Why do we need this restriction? Why can't leaves have arbitrary format strings?

Yeah, that's a good question. I mentioned it in the PR summary too:

At present, only one specifier per key in the format dictionary is allowed. This is because the whole point of JSON logging is to make logs easily machine-parseable. If you can include multiple formats in the same field, then you'll be right back to parsing those manually

The reason I chose to do this is to make the values sane. I didn't really see the value in doing something like:
{"protocol_and_duration": "%PROTOCOL% %DURATION%"}
because during log parsing you'd just have to manually parse that (with a regex or whatever) again.

The other use case would be to put plaintext in there, like:
{"protocol": "my protocol was %PROTOCOL%"}, which also doesn't seem particularly useful.

To me, it seemed saner to keep things simple. However this is an arbitrary restriction, and I'm not married to it. If you feel strongly that there should be multiple specifiers per key, let me know and I'll go ahead and lift that.

I guess my thinking was that the implementation of general purpose "%FOO% is %BAR% is already essentially free, you just reuse the existing access log formatter for leaf entries.

I can see legitimate uses here, e.g. where you are building semi-structured JSON for consumption by tools that don't care about the innermost structure.

It seems easier to just do this, as you have less validation to implement :D

I do see that argument, but I feel that having a forcing function to keep logs structured is net win. Additionally, it's easy to add multi-token support in the future, but hard to take it back.

Similar to disallowing nested dictionaries I propose we stick to single-token leaves for the time being, and then expand that if we see demand for it. As you mentioned it'll be a really simple change if we decide to do it.

In general, with the Envoy APIs we like to provide users flexibility and not to be too opinionated on their use. I generally would prefer if we relaxed the requirement, since it's not very hard to implement - we already have the formatter that should be able to do this in a single line.

Ok great, will tackle this

relax one formatter requiredment

htuch · 2018-10-11T22:13:14Z

source/common/access_log/access_log_formatter.cc

+    writer.String(pair.second.c_str());
+  }
+  writer.EndObject();
+  return fmt::format("{}\n", strbuf.GetString());


The alternative to using RapidJSON here is to build a google.protobuf.Struct and then do a Proto3 --> JSON conversion. I kind of prefer this, unless you want fine grained control over the output format.

I can look into this. Shouldn't be too complex, although there is a fair bit of boilerplate with constructing Proto3 objects (see the unit tests I wrote in config.cc). But I'll move forward with this anyway.

use protobuf to serialize instead of JSON

Turned out to be pretty straightforward

htuch · 2018-10-11T22:13:46Z

source/common/access_log/access_log_formatter.cc

+std::map<std::string, std::string> JsonFormatterImpl::to_map(
+    const Http::HeaderMap& request_headers, const Http::HeaderMap& response_headers,
+    const Http::HeaderMap& response_trailers, const StreamInfo::StreamInfo& stream_info) const {
+  std::map<std::string, std::string> output;


Do you need this to be ordered? Otherwise use unordered_map

No reason. Unordered should be fine

switch to unordered_map

htuch · 2018-10-11T22:14:25Z

source/common/access_log/access_log_formatter.h

+                     const Http::HeaderMap& response_trailers,
+                     const StreamInfo::StreamInfo& stream_info) const override;
+
+  std::map<std::string, std::string> to_map(const Http::HeaderMap& request_headers,


Does this need to be public? Should it just be a helper function in the .cc anonymous namespace?

Yeah not too happy about this either, the reasons were:

it's kinda hard to test this otherwise. exposing the map function makes it easy to just compare dicts instead of going through a parse/unparse dance. this isn't a super strong reason though

If we ever support other structured log formats (like proto), then the idea is that we would just rely on this function to create a c++ map, and then each new format would be a simple "serializer" that writes that out (even the current json stuff can be thought of like that). In that case, testing the map function provides more peace of mind

If you feel strongly though, lmk and I'm happy to make it anonymous

htuch · 2018-10-11T22:15:09Z

source/extensions/access_loggers/file/config.cc

+  if (fal_config.access_log_format_case() == envoy::config::accesslog::v2::FileAccessLog::kFormat ||
+      fal_config.access_log_format_case() ==
+          envoy::config::accesslog::v2::FileAccessLog::ACCESS_LOG_FORMAT_NOT_SET) {
+    if (fal_config.format().empty()) {


Why is the logic for non-JSON different to before here?

It's not strictly different, it has just been moved inside the check for
if (fal_config.access_log_format_case() == envoy::config::accesslog::v2::FileAccessLog::kFormat || fal_config.access_log_format_case() == envoy::config::accesslog::v2::FileAccessLog::ACCESS_LOG_FORMAT_NOT_SET)

i.e:

If the format key was provided, or neither key was provided, fall back to the old logic
else if the json_format key was provided, use the json logic
else something bad happened, bail

htuch · 2018-10-11T22:15:30Z

source/extensions/access_loggers/file/config.cc

@@ -37,6 +51,18 @@ ProtobufTypes::MessagePtr FileAccessLogFactory::createEmptyConfigProto() {

 std::string FileAccessLogFactory::name() const { return AccessLogNames::get().File; }

+std::map<const std::string, const std::string>
+FileAccessLogFactory::convert_json_format_to_map(ProtobufWkt::Struct json_format) {


convertJsonFormatToMap, same other comemnts as above.

case change convertJsonFormatToMap

htuch · 2018-10-11T22:16:12Z

test/extensions/access_loggers/file/config_test.cc

+  envoy::config::accesslog::v2::FileAccessLog fal_config;
+  fal_config.set_path("/dev/null");
+
+  auto json_format = new ProtobufWkt::Struct;


Probably don't want to be doing any raw new here, I think either make it stack allocated or a make_unique.

I was having some trouble with that earlier, will give it another try

So as far as I can tell, calling set_allocated_* with a non-null pointer causes protobuf to take ownership of that object, including releasing the object later (https://developers.google.com/protocol-buffers/docs/reference/cpp-generated#embeddedmessage). So I think using new is actually ok here.

Indeed, if I switch to using a unique_ptr and pass it in with .get(), I get a crash, presumably because the object gets freed twice, once by protobuf and the other by c++. It does work fine if you use make_unique and then .release() the pointer when passing it to set_allocated. I'm happy to do that if you want, although functionally, the effect is the same. Other option is to leave a comment on the call to new explaining why this is ok here.

The usual pattern is to have protobuf do this for you by reordering and doing a fal_config.mutable_allocated_json_format() first.

Figured this out. Thanks for the tip, that was the missing piece

Signed-off-by: Aaltan Ahmad <[email protected]>

aa-stripe · 2018-10-11T22:54:37Z

Thanks for the feedback @htuch! I agree with most of it, and I'll be updating the code shortly. Left a couple of explanations as well. In the meantime I've just some docs changes.

Signed-off-by: Aaltan Ahmad <[email protected]>

htuch

Thanks, taken another pass at it. Looks like it's getting into shape.

htuch · 2018-10-16T14:29:36Z

docs/root/configuration/access_log.rst

+* Each value in the dictionary can only contain one command operator.
+
+
+Command Operators


FWIW, I think "command operator" is not the right terminology, but I see it was existing before this PR. I probably would have used "format expressions" or the like.

I'll see if it's pretty straightforward to change this. I personally prefer "format specifiers" since it lines up with the C terminology

I left this as is for now

Sure, this was more just a non-actionable observation.

htuch · 2018-10-16T14:29:55Z

include/envoy/access_log/access_log.h

 */
 class Formatter {
 public:
  virtual ~Formatter() {}

+  /**
+   * Return a formatted access log line.
+   * @param request_headers supplies thte request headers.


Nit: thte typo

htuch · 2018-10-16T14:30:03Z

include/envoy/access_log/access_log.h

+
+  /**
+   * Extract a value from the provided headers/trailers/stream.
+   * @param request_headers supplies thte request headers.


This got copy/pasted :)

Argh! Fixed

htuch · 2018-10-16T14:30:37Z

include/envoy/access_log/access_log.h

+   * @param response_headers supplies the response headers.
+   * @param response_trailers supplies the response trailers.
+   * @param stream_info supplies the stream info.
+   * @return a string containing the complete formatted access log line.


Nit: FWIW @return std::string containing.. is better Envoy Doxygen style.

htuch · 2018-10-16T14:31:46Z

include/envoy/access_log/access_log.h

@@ -71,11 +71,20 @@ typedef std::shared_ptr<Instance> InstanceSharedPtr;

 /**
 * Interface for access log formatter.
+ * Formatters combine the output of FormatterProviders into a log output line.


Looking at this interface, it's a bit unclear how they do this combining; they don't take FormatterProviders as arguments to format..

Agreed. The combining takes place in the subclass which isn't entirely clear at this level. I changed to Formatters provide a complete access log output line for the given headers/trailers/stream.

htuch · 2018-10-16T14:32:44Z

source/common/access_log/access_log_formatter.cc

+                                      const Http::HeaderMap& response_headers,
+                                      const Http::HeaderMap& response_trailers,
+                                      const StreamInfo::StreamInfo& stream_info) const {
+  auto output_map = this->toMap(request_headers, response_headers, response_trailers, stream_info);


Nit: const auto

htuch · 2018-10-16T14:33:38Z

source/common/access_log/access_log_formatter.cc

+  std::string log_line;
+  auto conversion_status = ProtobufUtil::MessageToJsonString(output_struct, &log_line);
+  if (!conversion_status.ok()) {
+    throw EnvoyException(


We can't safely throw exceptions at runtime on the data path in Envoy, only during construction (config ingestion). Otherwise, Envoy will behave badly. We should have a more graceful failure in this scenario.

Cool, which do you prefer:

Return an empty string

Return a string containing something to the effect of Error converting access log output to JSON

pointer shenanigans to return an error code (I'm against this personally)

Probably JSON conversion error, it will be easier to understand WTF happened when this fails.

htuch · 2018-10-16T14:34:17Z

source/common/access_log/access_log_formatter.h

+                     const StreamInfo::StreamInfo& stream_info) const override;
+
+  std::unordered_map<std::string, std::string>
+  toMap(const Http::HeaderMap& request_headers, const Http::HeaderMap& response_headers,


If you want this accessible in tests, I suggest creating a test peer. I'm not sure why we need to test this internal behavior though, I'd skip it; it's pretty trivial.

I took another look at this. If we stop using this function to test and make it private, then we'll be relying on either

parsing the JSON string in order to do our asserts or

hardcoding an expected JSON string and comparing the raw string

I'll attempt 1, and see how it goes. I suppose it's a more accurate test anyway. I was worried 2 might be susceptible to the order of keys changing and therefore randomly failing tests

yep, 1 sounds good!

htuch · 2018-10-16T14:40:01Z

source/common/access_log/access_log_formatter.h

+
+private:
+  std::vector<FormatterProviderPtr> providers_;
+  std::map<const std::string, Envoy::AccessLog::FormatterProviderPtr> json_output_format_;


Can you add a comment everywhere there is std::map indicating why order matters (or switch to unordered_map)?

Yeah. Will do

Order didn't matter here either, changed to unordered

aa-stripe · 2018-10-16T18:23:21Z

Thanks for the valuable comments. I will upload changes soon

aa-stripe · 2018-10-16T18:24:49Z

I do have one question for you here: #4693 (comment)

Signed-off-by: Aaltan Ahmad <[email protected]>

…sion failure Signed-off-by: Aaltan Ahmad <[email protected]>

Signed-off-by: Aaltan Ahmad <[email protected]>

htuch

LGTM, just some nits and test structure suggestions.

htuch · 2018-10-17T23:05:53Z

docs/root/configuration/access_log.rst

+* Each value in the dictionary can only contain one command operator.
+
+
+Command Operators


Sure, this was more just a non-actionable observation.

htuch · 2018-10-17T23:07:32Z

source/common/access_log/access_log_formatter.cc

+                                      const Http::HeaderMap& response_trailers,
+                                      const StreamInfo::StreamInfo& stream_info) const {
+  const auto output_map =
+      this->toMap(request_headers, response_headers, response_trailers, stream_info);


Nit: you should be able to just write toMap rather than this->toMap. Personally I would inline toMap since it is now private, but up to you.

Removed the this->

htuch · 2018-10-17T23:09:31Z

test/common/access_log/access_log_formatter_test.cc

@@ -382,6 +382,210 @@ TEST(AccessLogFormatterTest, startTimeFormatter) {
  }
 }

+TEST(AccessLogFormatterTest, JsonFormatterTest) {


Can you break these into seperate TESTs. It's best to have distinct tests to control reruns and have finer grained visibility into specific failures. Also, can you add a one line // description above each?

Sure! Was following the style of the file, will break them up now

htuch · 2018-10-17T23:09:41Z

test/common/access_log/access_log_formatter_test.cc

+    StreamInfo::MockStreamInfo stream_info;
+    Http::TestHeaderMapImpl request_header{};
+    Http::TestHeaderMapImpl response_header{};
+    Http::TestHeaderMapImpl response_trailer{};


Nit: {} not needed for these.

htuch · 2018-10-17T23:11:06Z

test/extensions/access_loggers/file/config_test.cc

+                            "Didn't find a registered implementation for name: 'INVALID'");
+}
+
+TEST(FileAccessLogConfigTest, FileAccessLogJsonConversionTest) {


htuch · 2018-10-17T23:11:43Z

test/extensions/access_loggers/file/config_test.cc

+    string_value.set_string_value(std::string{"some_nested_value"});
+
+    ProtobufWkt::Value struct_value;
+    (*struct_value.mutable_struct_value()->mutable_fields())[std::string{"some_nested_key"}] =


Nit: do you need these std::string conversions?

Good catch! I must have been doing something else wrong earlier when I put them in. Removed

htuch · 2018-10-17T23:12:24Z

test/common/access_log/access_log_formatter_test.cc

+    JsonFormatterImpl formatter(key_mapping);
+
+    const auto parsed = Json::Factory::loadFromString(
+        formatter.format(request_header, response_header, response_trailer, stream_info));


Can you factor this repeated match pattern into a helper?

Signed-off-by: Aaltan Ahmad <[email protected]>

htuch

Thanks @aa-stripe!

aa-stripe · 2018-10-18T23:16:09Z

Thank you!

mattklein123 · 2018-10-19T02:31:26Z

@aa-stripe if possible can you do a follow up PR to add a release note? Thank you!

Add a new config option under access_log called json_format. This is a single level dictionary that contains strings as keys, and envoy access log format specifiers (such as %PROTOCOL%) as values. The specifiers will be replaced with actual values at logging time. I call this dictionary the "format dictionary" (as opposed to "format string"). You can specify only one of format (format string) or json_format (format dictionary). If neither are there, we fall back to the default string format. Add the correct plumbing inside the configuration parsing to handle this. Add a new access log formatter class that is instantiated with the format dictionary. It maintains the mapping of dictionary keys to loggers Create a new class called FormatterProvider, to distinguish things that actually extract the information from a request. The things that combine together a bunch of FormatterProviders are still called Formatters. This is primarily a semantic/naming difference, but imo these are two conceptually separate things. There is, however no API difference, and if people are truly opposed to this, I could just merge them back into one Formatter class. This also provides a better foundation for adding more log formats in the future. At present, only one specifier per key in the format dictionary is allowed. This is because the whole point of JSON logging is to make logs easily machine-parseable. If you can include multiple formats in the same field, then you'll be right back to parsing those manually At present, only top-level keys are allowed in the format dictionary. This is validated at config load time. In the future, we can expand this to have nested dictionaries. Risk Level: Low. It's an optional feature that has to be explicitly enabled. Testing: Unit testing for the actual formatter, and config load. Also manually tested using an example config file. Docs: Amended Access Log docs to create a notion of "Format Strings" and "Format Dictionaries". Put things that are common to access logs in general under "Format Rules", and then distinguished how strings and dictionaries are different. Called out restrictions on format dictionaries Added protobuf comments for format and json_format Signed-off-by: Aaltan Ahmad <[email protected]> Signed-off-by: Yang Song <[email protected]>

aa-stripe added 15 commits October 11, 2018 13:36

Initial commit

bfe7744

Signed-off-by: Aaltan Ahmad <[email protected]>

Minor fixes, remove a unique_ptr

031377f

Signed-off-by: Aaltan Ahmad <[email protected]>

Run clang-format on source/common/access_log

1c754a5

Signed-off-by: Aaltan Ahmad <[email protected]>

Run clang-format on source/extensions/access_loggers/file

f604562

Signed-off-by: Aaltan Ahmad <[email protected]>

Run clang-format on test/common/access_log

cbdc6dd

Signed-off-by: Aaltan Ahmad <[email protected]>

Run clang-format on test/extensions/access_loggers/file and switch fr…

8db57a7

…om google::protobuf to Protobuf Signed-off-by: Aaltan Ahmad <[email protected]>

Run clang-format on include/envoy/access_log

6af83d5

Signed-off-by: Aaltan Ahmad <[email protected]>

Update exception message in config load

bded5f2

Signed-off-by: Aaltan Ahmad <[email protected]>

Switch to using ProtobufWkt

ffd3358

Signed-off-by: Aaltan Ahmad <[email protected]>

Fix some format and ProtobufWkt issues

7c777d3

Signed-off-by: Aaltan Ahmad <[email protected]>

Adopt RequestInfo -> StreamInfo in the JSON logging code

ed1d0d8

Signed-off-by: Aaltan Ahmad <[email protected]>

More format fixes

4c7e000

Signed-off-by: Aaltan Ahmad <[email protected]>

More format fixes

216814f

Signed-off-by: Aaltan Ahmad <[email protected]>

More format fixes

777bd96

Signed-off-by: Aaltan Ahmad <[email protected]>

More format fixes - header reordering

9064213

Signed-off-by: Aaltan Ahmad <[email protected]>

aa-stripe force-pushed the aa-stripe/2692-json-logging-second-try branch from 864f205 to 9064213 Compare October 11, 2018 20:36

aa-stripe mentioned this pull request Oct 11, 2018

access log: add JSON logging mode #4681

Closed

aa-stripe changed the title ~~Aa stripe/2692 json logging second try~~ access log: add JSON logging mode Oct 11, 2018

Fix formatting issues

1d23323

Signed-off-by: Aaltan Ahmad <[email protected]>

htuch self-assigned this Oct 11, 2018

htuch suggested changes Oct 11, 2018

View reviewed changes

aa-stripe added 2 commits October 11, 2018 15:22

Add docs for JSON logging

cc0090e

Signed-off-by: Aaltan Ahmad <[email protected]>

Format fixes in file.proto

ccaab7c

Signed-off-by: Aaltan Ahmad <[email protected]>

aa-stripe added 4 commits October 11, 2018 17:07

Update comments in access_log.h

234a683

Signed-off-by: Aaltan Ahmad <[email protected]>

Switch to unordered_map

9f78381

Signed-off-by: Aaltan Ahmad <[email protected]>

Case change convertJsonFormatToMap

f2b1173

Signed-off-by: Aaltan Ahmad <[email protected]>

Describe some test cases

633365a

Signed-off-by: Aaltan Ahmad <[email protected]>

htuch suggested changes Oct 16, 2018

View reviewed changes

aa-stripe added 11 commits October 16, 2018 15:09

Fix some wording

6273f4b

Signed-off-by: Aaltan Ahmad <[email protected]>

Make toMap private and compare parsed JSON in unit tests

588a28f

Signed-off-by: Aaltan Ahmad <[email protected]>

Log an error instead of throwing an exception on proto -> json conver…

85e05f9

…sion failure Signed-off-by: Aaltan Ahmad <[email protected]>

Switch to unordered_map for config.cc

a3e4410

Signed-off-by: Aaltan Ahmad <[email protected]>

Remove single token restriction

91b1b80

Signed-off-by: Aaltan Ahmad <[email protected]>

Remove wording about one token limitation

0bf86f8

Signed-off-by: Aaltan Ahmad <[email protected]>

Formatting

5aa43e2

Signed-off-by: Aaltan Ahmad <[email protected]>

Formatting

d8a51e9

Signed-off-by: Aaltan Ahmad <[email protected]>

Formatting

b87f2a4

Signed-off-by: Aaltan Ahmad <[email protected]>

use const auto

6ef6456

Signed-off-by: Aaltan Ahmad <[email protected]>

Formatting

1ffc475

Signed-off-by: Aaltan Ahmad <[email protected]>

htuch suggested changes Oct 17, 2018

View reviewed changes

aa-stripe added 5 commits October 18, 2018 13:03

Break up tests and remove this->

e92a886

Signed-off-by: Aaltan Ahmad <[email protected]>

Break up config tests into separate test cases

252df75

Signed-off-by: Aaltan Ahmad <[email protected]>

Remove std::string conversion

a060f50

Signed-off-by: Aaltan Ahmad <[email protected]>

Refactor json map assertions into a helper

e7ca4a7

Signed-off-by: Aaltan Ahmad <[email protected]>

Formatting fixes

d0221c8

Signed-off-by: Aaltan Ahmad <[email protected]>

htuch approved these changes Oct 18, 2018

View reviewed changes

htuch merged commit de03926 into envoyproxy:master Oct 18, 2018

aa-stripe mentioned this pull request Oct 19, 2018

Add release notes for json logging and fix a typo #4784

Merged

This was referenced Oct 19, 2018

add field to global config for setting the access log format to text or json istio/api#660

Merged

update envoy SHA to point to json access log format functionality istio/proxy#1999

Merged

GregHanson mentioned this pull request Nov 12, 2018

Missing new line between JSON access log entries #5018

Closed

ghostsquad mentioned this pull request Jun 17, 2020

JSON Application Logs #11610

Open

		* Each value in the dictionary can only contain one command operator.


		Command Operators

access log: add JSON logging mode #4693

access log: add JSON logging mode #4693

Conversation

aa-stripe commented Oct 11, 2018 • edited Loading

aa-stripe commented Oct 11, 2018 • edited Loading

aa-stripe commented Oct 11, 2018

htuch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aa-stripe Oct 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aa-stripe Oct 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aa-stripe Oct 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aa-stripe Oct 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aa-stripe Oct 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aa-stripe Oct 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aa-stripe Oct 11, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aa-stripe Oct 12, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aa-stripe commented Oct 11, 2018

htuch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aa-stripe commented Oct 16, 2018

aa-stripe commented Oct 16, 2018

htuch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aa-stripe commented Oct 11, 2018 •

edited

Loading

aa-stripe commented Oct 11, 2018 •

edited

Loading

aa-stripe Oct 11, 2018 •

edited

Loading

aa-stripe Oct 11, 2018 •

edited

Loading

aa-stripe Oct 11, 2018 •

edited

Loading

aa-stripe Oct 11, 2018 •

edited

Loading

aa-stripe Oct 11, 2018 •

edited

Loading

aa-stripe Oct 11, 2018 •

edited

Loading

aa-stripe Oct 11, 2018 •

edited

Loading

aa-stripe Oct 12, 2018 •

edited

Loading