Stop using deprecated fields in hcm#19474
Conversation
howardjohn
left a comment
There was a problem hiding this comment.
@dougreid can you check the tracing change?
| httpOpts: &httpListenerOpts{ | ||
| rds: routeName, | ||
| useRemoteAddress: true, | ||
| direction: http_conn.HttpConnectionManager_Tracing_EGRESS, // viewed as from gateway to internal |
There was a problem hiding this comment.
replacement is TrafficDirection on listener - which I think we are already setting.
There was a problem hiding this comment.
Can we validate this is true and we are setting this everywhere? otherwise we are going to break tracing and no one will notice for months
There was a problem hiding this comment.
otherwise we are going to break tracing and no one will notice for months
It sounds like we need a test that validates/ensures that we have set the tracing config correctly and any conscious changes, need updating that test case?
There was a problem hiding this comment.
Do we have any test covering this?
There was a problem hiding this comment.
TrafficDirection definitely overrides this (I did this change).
I can't say that we set traffic direction everywhere (we should).
There was a problem hiding this comment.
Reg, Setting traffic direction on Listeners I verified some time back and ensured that it is set on all listeners - #18833
Do we have any test covering this?
Not sure. If it is not, I think it may be a good idea to add one that validates that we have the config needed for tracing functionality.
|
@douglas-reid Please take a look if there is any influence on tracing |
|
/retest |
No description provided.