@Ladsgroup we should follow up with Traffic to make sure that ATS-TLS adds the client's source port in one HTTP header, since Varnish (and varnishkafka) are behind it (so the source port will always be the ATS one). ATS-TLS is the new nginx (and soon to be replaced again!)

In T271953#6748341, @elukey wrote:

@Ladsgroup we should follow up with Traffic to make sure that ATS-TLS adds the client's source port in one HTTP header, since Varnish (and varnishkafka) are behind it (so the source port will always be the ATS one). ATS-TLS is the new nginx (and soon to be replaced again!)

Thanks. Just curiosity. What is going to be replaced with? nginx- again?

@ema quick question - is the client src port something that we could pass from ATS-TLS to Varnish frontend? Via HTTP header etc..

In T271953#6749428, @Ladsgroup wrote:

In T271953#6748341, @elukey wrote:

@Ladsgroup we should follow up with Traffic to make sure that ATS-TLS adds the client's source port in one HTTP header, since Varnish (and varnishkafka) are behind it (so the source port will always be the ATS one). ATS-TLS is the new nginx (and soon to be replaced again!)

Thanks. Just curiosity. What is going to be replaced with? nginx- again?

envoy may be the next TLS terminator, but the Traffic team will announce the choice once the review is done! :)

Hi! We'd like to add this to the X-Analytics header, if that's ok with everyone. This way we don't have to add a new field. Here's another task that's adding a debug flag this way: T263683, along with a patch for example: https://gerrit.wikimedia.org/r/c/operations/puppet/+/629735

It's okay for me, as long as I have a way to obtain the data, it works for me :D

Change 657296 had a related patch set uploaded (by Vgutierrez; owner: Vgutierrez):
[operations/puppet@production] ATS: provide client port information to varnish

https://gerrit.wikimedia.org/r/657296

In T271953#6755027, @elukey wrote:

@ema quick question - is the client src port something that we could pass from ATS-TLS to Varnish frontend? Via HTTP header etc..

It is, yes!

Change 657296 merged by Vgutierrez:
[operations/puppet@production] ATS: provide client port information to varnish

https://gerrit.wikimedia.org/r/657296

Reporting a chat with the Traffic team happened this morning:

• https://gerrit.wikimedia.org/r/657296 is needed since ATS-TLS doesn't really add any content to X-Analytics (it just creates X-Analytics-TLS), so we'll need to pass X-Client-Port to Varnish
• In Varnish we'll need to add VCL code to set the new parameters to X-Analytics, so that Varnishkafka will pick them up and we'll able to use them in Analytics-land.

In T271953#6761141, @elukey wrote:

• In Varnish we'll need to add VCL code to set the new parameters to X-Analytics, so that Varnishkafka will pick them up and we'll able to use them in Analytics-land.

Something like:

if (req.http.X-Client-Port) {
    set resp.http.X-Analytics = resp.http.X-Analytics + ";client_port=" + req.http.X-Client-Port;
}

Change 657416 had a related patch set uploaded (by Effie Mouzeli; owner: Effie Mouzeli):
[operations/puppet@production] varnish: include X-Client-Port in X-Analytics

https://gerrit.wikimedia.org/r/657416

Change 657416 abandoned by Effie Mouzeli:
[operations/puppet@production] varnish: include X-Client-Port in X-Analytics

Reason:
rebase probs

https://gerrit.wikimedia.org/r/657416

Change 658567 had a related patch set uploaded (by Effie Mouzeli; owner: Effie Mouzeli):
[operations/puppet@production] varnish: include X-Client-Port in X-Analytics

https://gerrit.wikimedia.org/r/658567

Change 658567 merged by Vgutierrez:
[operations/puppet@production] varnish: include X-Client-Port in X-Analytics

https://gerrit.wikimedia.org/r/658567

Both changed deployed by Valentin, I checked the client_port field in webrequest_text on Kafka and it works nicely. The debug header needs to be triggered by an external user, if we want to test it we need to ping @jijiki for a quick test :)

Since both new fields are logged into X-Analytics, there shouldn't be any change needed to Hive etc.. IIUC, but probably only a change in Druid's webrequest_128 to have X-Analytics' fields logged as map?

Debug header works, we tested it with @elukey:)

The pageview definition was changed to react to the debug header, and the change to load webrequest_128 is up for review (both linked to T273083).