elastic / apm

Elastic Application Performance Monitoring - resources and general issue tracking for Elastic APM.
https://www.elastic.co/apm
Apache License 2.0
384 stars 114 forks source link

Update Kubernetes pod UID parsing spec #678

Closed z1c0 closed 2 years ago

z1c0 commented 2 years ago
apmmachine commented 2 years ago

:green_heart: Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

#### Build stats * Start Time: 2022-09-19T05:48:00.114+0000 * Duration: 3 min 8 sec

z1c0 commented 2 years ago

LGTM! I think you make this PR ready for review.

Thanks, will do!

Can you provide a couple of examples for /proc/self/cgroup file lines that we can use for unit testing?

In apm-agent-dotnet, this unit test was added (id: 03752a671e744971a862edcee6195646-4015103728).

felixbarny commented 2 years ago

Can you provide a couple of examples for /proc/self/cgroup file lines that we can use for unit testing?

In apm-agent-dotnet, this unit test was added (id: 03752a671e744971a862edcee6195646-4015103728).

I think it makes sense to add a common json test spec in https://github.com/elastic/apm/tree/main/tests/agents/json-specs for this. I don't see this as blocking this PR, though.

z1c0 commented 2 years ago

Can you provide a couple of examples for /proc/self/cgroup file lines that we can use for unit testing?

In apm-agent-dotnet, this unit test was added (id: 03752a671e744971a862edcee6195646-4015103728).

I think it makes sense to add a common json test spec in https://github.com/elastic/apm/tree/main/tests/agents/json-specs for this. I think see this as blocking this PR, though.

Make sense, I wasn't aware this existed though :-) I'll update the test spec and (add a test in https://github.com/elastic/apm-agent-dotnet/issues/1805)