maskit commented on pull request #7511: URL: https://github.com/apache/trafficserver/pull/7511#issuecomment-777919851
Replying to the comment on #7491. > Because we already have a ton of fields, I understand the sentiment of trying to keep it fewer. But I still not sure overwriting the cqpv by negotiated ALPN id is a better approach. I'm not suggesting using ALPN ID for cqpv. I think we should not do it. > Compatibility The current value of HTTP/2 protocol, http/2, needs to be changed to h2. This involves every user who is using cqpv. My suggestion is to log "http/2" in cqpv field if HTTP/2 is used in a tunnel. So there would be no incompatibility. If it's logged as "http", I'd call it a bug and I don't think it's a something we want to keep. > Hiding some cases The approach hides some cases because cqpv needs to log protocol and version even if ALPN was not used. > > e.g. HTTP/1.1 over TCP, HTTP/1.1 over TLS without ALPN, and Blind Tunnel. ( the cqssa says - in these cases ) I merge this PR to roll forward. We can use a piece of code from this change in case we decide to go to the alt approach. What would be hidden? Do you mean you also need to log whether ALPN is used? I don't think you excluded NPN case on #7491. cqpv should have protocol name and version as long as ATS knows. If ATS doesn't know, it should log "-". There may be broken cases now, but that is the behavior I expect. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected]
