Label frames with container_id #274
Conversation
src/ddprof_process.cc
Outdated
| } | ||
| } | ||
| ++(it->second._sample_counter); | ||
| if(!force && (it->second._sample_counter) < k_nb_samples_container_id_lookup) { |
There was a problem hiding this comment.
This is something I'll want to talk about.
|
This PR will add a custom slice dimension using the container-id: |
The aim is to avoid a dependency on libdatadog for testing purposes.
Add a test for cgroup lookup
Change the lookup logic to be per PID
r1viollet
force-pushed
the
r1viollet/tag_containers
branch
from
600aab7 to
603604c
Compare
Clarify status of container-id field
I slightly changed the labels. They now read:
|
r1viollet
force-pushed
the
r1viollet/tag_containers
branch
from
280f212 to
8679d17
Compare
r1viollet
force-pushed
the
r1viollet/tag_containers
branch
from
8679d17 to
c857927
Compare
Fix error when returning ref to temp element
src/ddprof_process.cc
Outdated
| return {}; | ||
| } | ||
| } | ||
| container_id = "none"; |
There was a problem hiding this comment.
Ah, interesting. So our default is "unknown", but failure to process this upgrades it to "none". I think there's value in having a semantic distinction and I think these two values are as good as any other, but in the future we may want to be more descriptive.
There was a problem hiding this comment.
Thanks for highlighting that this was unclear. I added comments.
none means that it is operating outside a container.
|
CI was flaky due to a metric push failing. |
What does this PR do?
Add a label with container_id information
We won't lookup short lived processes.
Motivation
When viewing data from a host, we want to understand if our process is running within a container.
Refer to following issue: #220
Additional Notes
This introduces a ProcessHdr object as an attempt to move some of the responsibilities to this object.
I have a libdatadog PR open : DataDog/libdatadog#169
Though I will not wait for the libdatadog code. Most of the code would be the same.
How to test the change?
I added some unit tests that make sure we are able to read the container_id information.
An end to end test is still missing.