Adding static config resource detector for tracing #34944
Conversation
Signed-off-by: Juan Manuel Ollé <jolle@mulesoft.com>
|
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to |
Signed-off-by: Juan Manuel Ollé <jolle@mulesoft.com>
Signed-off-by: Juan Manuel Ollé <jolle@mulesoft.com>
Signed-off-by: Juan Manuel Ollé <jolle@mulesoft.com>
| private: | ||
| const envoy::extensions::tracers::opentelemetry::resource_detectors::v3:: | ||
| StaticConfigResourceDetectorConfig config_; |
There was a problem hiding this comment.
Rather than storing the whole proto config here, I think you could just to store the attributes by a flat_hash_map
| typed_config: | ||
| "@type": type.googleapis.com/envoy.extensions.tracers.opentelemetry.resource_detectors.v3.StaticConfigResourceDetectorConfig |
There was a problem hiding this comment.
The integration test is a little weird. I think at least we should add a case where valid attributes are work?
There was a problem hiding this comment.
Yes, I just take a look what the other 2 detectors were doing. But I agree the use case you mention is better
Signed-off-by: Juan Manuel Ollé <jolle@mulesoft.com>
Signed-off-by: Juan Manuel Ollé <jolle@mulesoft.com>
|
/retest |
| for (const auto& pair : attributes_) { | ||
| const std::string& key = pair.first; | ||
| const std::string& value = pair.second; |
There was a problem hiding this comment.
for(const auto& [key, value] : attributes_) would be simpler.
| ], | ||
| extension_names = ["envoy.tracers.opentelemetry.resource_detectors.static_config"], | ||
| deps = [ | ||
| "//source/exe:main_common_lib", |
There was a problem hiding this comment.
This is not a good practice to depend the whole main. You should figure out what lib is actually be depended and list the actually dependency here.
There was a problem hiding this comment.
true, sorry, I have copied it and it is not needed this lib at all.
Signed-off-by: Juan Manuel Ollé <jolle@mulesoft.com>
|
@wbpcode friendly ping |
|
Ping @wbpcode PTAL |
| EXPECT_TRUE(resource.attributes_.empty()); | ||
| } | ||
|
|
||
| // Test detector with valid values in attributes config |
|
/wait |
Signed-off-by: Juan Manuel Ollé <jolle@mulesoft.com>
|
cc @juanmolle Hi, please merge main then we can try to merge it. :) |
Signed-off-by: Juan Manuel Ollé <jolle@mulesoft.com>
|
My apologies, I didn't understand your comment. |
Implementation for new trace resource detector.
This new Detector allows to use static config for resource attributes.
Risk Level: low
Testing: yes
Docs Changes: yes
Release Notes: yes
Platform Specific Features: No