envoy Add missing upstream string formatters

%UPSTREAM_{PEER,LOCAL}_{URI,DNS,IP}_SAN% were not implemented; add them. Given that we're not too far from when v1.30 was cut, would it be possible to backport this to 1.30?

Apr 30 '24 00:04 keithmattix

Given that we're not too far from when v1.22 was cut, would it be possible to backport this to 1.30?

Do you want to backport to v1.30? If so, it is probably possible, and will require a subsequent PR to that release branch.

Apr 30 '24 12:04 adisuissa

Do you want to backport to v1.30? If so, it is probably possible, and will require a subsequent PR to that release branch.

Sure, I'm happy to submit the 1.30 patch

Apr 30 '24 13:04 keithmattix

Thank you for this contribution! The change in source/common/formatter/stream_info_formatter.cc seems pretty large compared to what this PR should achieve. It seems to be mainly due to a format related change. Can you make sure that the right formatting tools are used? /wait

@adisuissa I'm running ./tools/local_fix_format.sh; is there another tool I should be using?

Apr 30 '24 13:04 keithmattix

_format

Can you please try running it using these instructions?

May 06 '24 17:05 adisuissa

Quick check why CI failed, and I see:

 [ RUN      ] SubstitutionFormatterTest.streamInfoFormatterWithSsl
                  test/common/formatter/substitution_formatter_test.cc:1861: Failure
                  Expected equality of these values:
                  "san"
                     Which is: 0x22d019
                  upstream_format.formatWithContext({}, stream_info)
                    Which is: (nullopt)

Can you locally run bazel test //test/common/formatter:substitution_formatter_test

It should fail in your local env as well and should be straightforward to debug why it happens.

May 13 '24 15:05 cpakulski

Looks like a flake in the udp tests?

/retest envoy-presubmit /retest envoy-presubmit (Checks (Linux x64) Linux x64 compile_time_options)

May 13 '24 21:05 keithmattix

@cpakulski @adisuissa thanks for your patience on this review; still learning the ins and outs of the devtools. Formatting and tests should be good to go now!

May 13 '24 22:05 keithmattix

/wait

May 14 '24 15:05 adisuissa

@envoyproxy/senior-maintainers assignee is @zuercher

:cat:

Caused by: a https://github.com/envoyproxy/envoy/pull/33857#pullrequestreview-2063812465 was submitted by @adisuissa.

see: more, trace.

May 17 '24 15:05 repokitteh-read-only[bot]

@keithmattix I think you should also update changelogs/current.yaml.

Does it need to be something different than what I have here?

May 17 '24 19:05 keithmattix

Ah, sorry. I missed it. All good. Thanks!

May 17 '24 19:05 cpakulski

Sorry this needs a merge. I'll try to keep a better eye on it and get it merged.

May 23 '24 00:05 zuercher

No worries @zuercher - it should be caught up now

May 23 '24 20:05 keithmattix

/retest envoy-presubmit /retest envoy-presubmit (Linux arm64 Build and test)

May 24 '24 14:05 keithmattix

FYI we found a way around this and don't need to backport

May 29 '24 22:05 keithmattix