Turn on v2 telemetry on by default for http and tcp#20258
Turn on v2 telemetry on by default for http and tcp#20258istio-testing merged 2 commits intoistio:masterfrom
Conversation
6c6f6db to
b2e67f7
Compare
There was a problem hiding this comment.
v2 enabled already implies tcp v1 I think?
There was a problem hiding this comment.
Adding this option, so that users can go back to behavior of 1.4
In 1.5(pending approval)=>
v2.enabled => tcp and http v2
if user wants to go back to 1.4 behavior:
v1.tcp.enabled => tcp v1 and http v2
There was a problem hiding this comment.
What if they set v1.enabled and v2.enabled? Today this is how they get tcp v1 and http v2
There was a problem hiding this comment.
oh ok.. in that case we dont need a new value
we can say as follows:
v2.enabled, v1.disabled : v2 http, v2 tcp
v2.enabled, v1.enabled: v2 http, v1 tcp
v2.disabled, v1.disabled: no telemetry
v2.disabled, v1.enabled: v1 http, v1 tcp
@mandarjog , @bianpengyuan : what do you guys think too?
There was a problem hiding this comment.
I am ok with the above, but also open to other ideas
There was a problem hiding this comment.
Can we add this condition to the preceding clause?
There was a problem hiding this comment.
Removed as we are not adding new TCP specific value
There was a problem hiding this comment.
Is this trying to redefine this key ? It may work with parsing but would be confusing
There was a problem hiding this comment.
No, just setting it empty
463e7db to
d8ac79c
Compare
2e7e9c5 to
b812b1d
Compare
fed68a5 to
32958f9
Compare
There was a problem hiding this comment.
is this not a wasm filter? Do we add this in addition to the http filter?
There was a problem hiding this comment.
oh nevermind the http one is only on HCM
There was a problem hiding this comment.
nit/question: do we actually want this to be a separate envoyfilter from http? I think internally we merge anyways so there isn't perf issues either way
There was a problem hiding this comment.
this is added to network filter chain for tcp and the other one to HCM
|
/cherrypick release-1.5 |
|
@howardjohn: once the present PR merges, I will cherry-pick it on top of release-1.5 in a new PR and assign it to you. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Turn on v2 telemetry on by default for http and tcp THIS IS PENDING TOC APPROVAL For TCP this will use Alpn based prefixed tunelling metadata exchange -> pending istio#20126 Followed @howardjohn's PR istio#20225 for this. Fixed based on feedback Fix bad rebase Trying to fix bas rebase again Trying to fix bas rebase again Trying to fix bas rebase again fix assets file Running Make Gen Running Make Gen after changes to values.yaml Set telemetry component to false Add EnvoyFilter for TCP Run Make gen again Enable mixer in mixer tests Fix Unit test
9c68461 to
d8ba0ba
Compare
|
@mandarjog , @bianpengyuan : Can you please take a look at this pr? |
|
@gargnupur: The following test failed, say
DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
@howardjohn , @mandarjog : looks like I am not authorized to merge this. Can you please help merge this PR? |
|
@gargnupur see https://github.com/istio/istio/wiki/Working-with-Prow#why-isnt-my-pr-merging this is expected and just needs to wait |
|
@howardjohn: #20258 failed to apply on top of branch "release-1.5": DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
* add option Turn on v2 telemetry on by default for http and tcp THIS IS PENDING TOC APPROVAL For TCP this will use Alpn based prefixed tunelling metadata exchange -> pending istio#20126 Followed @howardjohn's PR istio#20225 for this. Fixed based on feedback Fix bad rebase Trying to fix bas rebase again Trying to fix bas rebase again Trying to fix bas rebase again fix assets file Running Make Gen Running Make Gen after changes to values.yaml Set telemetry component to false Add EnvoyFilter for TCP Run Make gen again Enable mixer in mixer tests Fix Unit test * Run make gen (cherry picked from commit a24f595)
* add option Turn on v2 telemetry on by default for http and tcp THIS IS PENDING TOC APPROVAL For TCP this will use Alpn based prefixed tunelling metadata exchange -> pending #20126 Followed @howardjohn's PR #20225 for this. Fixed based on feedback Fix bad rebase Trying to fix bas rebase again Trying to fix bas rebase again Trying to fix bas rebase again fix assets file Running Make Gen Running Make Gen after changes to values.yaml Set telemetry component to false Add EnvoyFilter for TCP Run Make gen again Enable mixer in mixer tests Fix Unit test * Run make gen (cherry picked from commit a24f595) Co-authored-by: Nupur Garg <37600866+gargnupur@users.noreply.github.com>
Please provide a description for what this PR is for.
Turn on v2 telemetry on by default for http and tcp
THIS IS PENDING TOC APPROVAL
For TCP this will use Alpn based prefixed tunelling metadata exchange -> pending #20126
Followed @howardjohn's PR #20225 for this.
And to help us figure out who should review this PR, please
put an X in all the areas that this PR affects.
[ ] Configuration Infrastructure
[ ] Docs
[X ] Installation
[ ] Networking
[ ] Performance and Scalability
[X ] Policies and Telemetry
[ ] Security
[ ] Test and Release
[ ] User Experience
[ ] Developer Infrastructure