Resource Detection processor #309
Conversation
Codecov Report
@@ Coverage Diff @@
## master #309 +/- ##
==========================================
+ Coverage 83.42% 83.78% +0.36%
==========================================
Files 171 178 +7
Lines 9243 9504 +261
==========================================
+ Hits 7711 7963 +252
- Misses 1202 1209 +7
- Partials 330 332 +2
Continue to review full report at Codecov.
|
james-bebbington
force-pushed
the
resourcedetection-processor
branch
3 times, most recently
from
b6e8688 to
75951e4
Compare
james-bebbington
changed the title
james-bebbington
force-pushed
the
resourcedetection-processor
branch
from
bbbfc25 to
2bc57aa
Compare
james-bebbington
force-pushed
the
resourcedetection-processor
branch
from
2bc57aa to
001ea16
Compare
james-bebbington
force-pushed
the
resourcedetection-processor
branch
2 times, most recently
from
9cca643 to
45e5208
Compare
|
|
||
| value := match[2] | ||
| var err error | ||
| if value, err = url.QueryUnescape(value); err != nil { |
There was a problem hiding this comment.
I didn't quite follow from the spec, is the purpose of the query encoding so that = and , are being used as special separator values and the encoding allows those values being used in the actual resource key/values not to be confused with the separators?
There was a problem hiding this comment.
I was going off this PR / comment.
I believe the intention was to simplify the definition of how to encode resources in an environment variable by borrowing (i.e. referencing) the spec that is already defined for correlation context, since these are basically the same format. The reason why we need this isn't explicitly called out in that PR, but I assume it's for the reason you mentioned.
Even though that PR probably won't get merged, there seemed to be support for using this definition for resources supplied in environment variables. I included a similar statement in the resource detection OTEP.
james-bebbington
force-pushed
the
resourcedetection-processor
branch
4 times, most recently
from
7ad1de5 to
2df6894
Compare
james-bebbington
force-pushed
the
resourcedetection-processor
branch
3 times, most recently
from
83be09c to
69d94cf
Compare
james-bebbington
force-pushed
the
resourcedetection-processor
branch
3 times, most recently
from
37e6ba8 to
e116534
Compare
james-bebbington
force-pushed
the
resourcedetection-processor
branch
4 times, most recently
from
0ded499 to
bfadd02
Compare
james-bebbington
force-pushed
the
resourcedetection-processor
branch
5 times, most recently
from
c784daf to
2e512ee
Compare
james-bebbington
force-pushed
the
resourcedetection-processor
branch
from
2e512ee to
500a020
Compare
| }) | ||
|
|
||
| return mp | ||
| } |
There was a problem hiding this comment.
@bogdandrutu - I had to write this function to be able to print Resource Attributes to the log. It feels like it would be useful to have a helper function to make this kind of debugging easier; maybe an implemention ofString()?
There was a problem hiding this comment.
Do you want to do this in a separate PR or here?
* Add resourcedetection processor * Add env and gce resource detectors * Resource Detection Processor: increase test coverage & fix README * Cache detected resource against processor name to avoid running resource detection code more than once * Refactor resources detection processor to use resource provider
Link to tracking Issue:
open-telemetry/opentelemetry-collector#871
Description:
Adds a new processor that automatically detects the resource based on the configured set of detectors. Initial support has been added for detecting resources from an environment variable & from GCE metadata. The user can configure to override or preserve existing resource information.
Sorry this PR is a little large. I separated it into 2 commits: