-
Notifications
You must be signed in to change notification settings - Fork 483
[CONFIGURATION] File configuration - misc model #3504
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for opentelemetry-cpp-api-docs canceled.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3504 +/- ##
==========================================
- Coverage 89.96% 89.94% -0.02%
==========================================
Files 219 219
Lines 7051 7051
==========================================
- Hits 6343 6341 -2
- Misses 708 710 +2 🚀 New features to boost your workflow:
|
|
||
#pragma once | ||
|
||
#include <map> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
map can be removed.
namespace configuration | ||
{ | ||
|
||
enum enum_otlp_http_encoding : std::uint8_t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a comment indicating the intended deviation from the style guide. Perhaps link to this #3467 (comment)?
|
||
#pragma once | ||
|
||
#include <map> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
map can be removed
std::unique_ptr<AttributesConfiguration> attributes; | ||
std::unique_ptr<DetectorsConfiguration> detectors; | ||
std::string schema_url; | ||
std::string attributes_list; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a required notation for the string of attributes? "key=value;..." ?
|
||
// YAML-SCHEMA: schema/resource.json | ||
// YAML-NODE: AttributeNameValue | ||
// FIXME: Name/Value/Type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you share more on the FIXME's? Will these be addressed in later PRs as part of the initial roll out of the yaml configuration feature or left for a later phase?
Contributes to #2481
This is a partial fix, that implements supporting declarations.
Changes
Please provide a brief description of the changes here.
For significant contributions please make sure you have completed the following items:
CHANGELOG.md
updated for non-trivial changes