Envoy: v2 API Access Log Filter Schema is incorrect

[winguse]

v2 API Access Log Filter Schema is incorrect

This is the V1 config:

              filter:
                type: logical_and
                filters:
                - type: logical_or
                  filters:
                  - type: status_code
                    op: ">="
                    value: 400
                  - type: status_code
                    op: "="
                    value: 0
                  - type: duration
                    op: ">="
                    value: 2000
                  - type: traceable_request
                - type: not_healthcheck

This is one V2 config I can think of:

            filter:
              and_filter:
                filters:
                  - not_health_check_filter: {}
                  - or_filter:
                      filters:
                        - status_code_filter:
                            comparison:
                              op: GE
                              value:
                                default_value: 300
                                runtime_key: "" # What's this? And this is required.
                        - status_code_filter:
                            comparison:
                              op: EQ
                              value:
                                default_value: 0
                                runtime_key: "" # What's this?
                        - duration_filter:
                            comparison:
                              op: GE
                              value:
                                default_value: 2000
                                runtime_key: "" # What's this?
                        - traceable_filter: {}

As searching the code, I can see runtime_key is something like access_log.access_error.status.

And I think the problem here is incorrect use of RuntimeUInt32 inside ComparisonFilter.

RuntimeUInt32.runtime_key looks like is to extract information from log and fall back to
RuntimeUInt32.default_value.

[mattklein123]

@winguse what your question? Are you having a problem with a config that doesn't load? Or you don't know how to configure something?

[winguse]

@mattklein123 what value shall I set in runtime_key? no document talking about it. and from the comments, I feel RuntimeUInt32 is incorrect used.

[mattklein123]

@junr03 can you help answer @winguse question?

[junr03]

@winguse the runtime_key allows you to override the default value at runtime, without having to reload a bootstrap configuration. If you have no desire to let the value be runtime overridable then you can set the runtime_key to the empty string as you have done above. lmk if you have any more questions

[winguse]

@junr03 it’s not allowed to put empty string, envoy will complain about if it’s empty. And the document is confusing if I put this key inside status comparison filter, right ?

[junr03]

@winguse sorry I got confused. In fact yes, you will have to give those values a string with at least one byte. Now -- what I propose is that you give them a valid path in your runtime scheme, and in your runtime deployment dont create the files. This will then default to the specified default value, and if need be you'll be able to runtime control the values in the future

[winguse]

@junr03 could you help to give an example for access log filter in this case?

[mattklein123]

@junr03 what's the resolution this one? Is there a doc update needed or some other issue?

[junr03]

Sorry this dropped off my radar.

Here is the summary: the RuntimeUInt32 type requires a runtime key. This means that it is not possible to configure access logs with default values that are not runtime overridable.

There are two options:

1. data-plane-api deletes the runtime key requirement for the RuntimeUInt32 type
2. The configuration specifies a sensible runtime key _in case_ that the engineer wants to runtime override in the future

I recommented to @winguse to go with option 2.

So @winguse I dont have a concrete example for you other than pick a sensible path for where that key would live in your runtime system. Does that make sense?

[winguse]

Make sense, thanks @junr03

[troshko111]

I believe this should be reopened as a bug for these reasons:

• Nowhere in the docs or this thread it is explained what a "runtime key" even is
• Runtime key should be deprecated and a new optional field should be used as it makes no sense to have this as a required field
• Documentation should be updated to explain what a runtime key is, its behavior if set vs unset and an example usage

This is the current doc:

Runtime key to get value for comparison. This value is used if defined.

1) "Runtime key to get value" what is a runtime key?
2) This is a required field, hence what does this even mean "used if defined"? It's always defined

I understand this is a backward-incompatible change but Envoy has been historically ok with deprecating and removing things which make no sense and this does not look like a candidate for an exception.

In the meantime, can the docs be clarified to explain the notion of "runtime key" and suggest a workaround when one is not needed?

@junr03

[troshko111]

In case the docs aren't updated, I checked the code and the tests and this should (and seems to) be an acceptable work around (log http codes <= 199 || >= 300):

{
  "or_filter": {
    "filters": [
      {
        "status_code_filter": {
          "comparison": {
            "op": "LE",
            "value": {
              "default_value": 199,
              "runtime_key": "null"
            }
          }
        }
      },
      {
        "status_code_filter": {
          "comparison": {
            "op": "GE",
            "value": {
              "default_value": 300,
              "runtime_key": "null"
            }
          }
        }
      }
    ]
  }
}

Envoy always attempts to look up "null" since its non-empty (but it must be due to the bug in the proto schema) so there's minor overhead.