-
Notifications
You must be signed in to change notification settings - Fork 501
[SDK] Implementation of container resource as per semconv #3572
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
Changes from 1 commit
c740ddb
4bf2bfb
f075c93
86cebd6
00decf9
b84840f
edef26e
3742932
a495294
72565ec
f7b5e94
a836349
5ec872c
81624a3
4cebe35
a1778b9
926517f
bd7a71c
2546661
75c495c
220a5ad
439e92c
9e5bc55
9bc07e1
047e35c
a404c80
7cbcc59
0e7d011
fd93b32
b030cc9
018a1f8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
// Copyright The OpenTelemetry Authors | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
#pragma once | ||
|
||
#include <string> | ||
|
||
#include "opentelemetry/version.h" | ||
|
||
OPENTELEMETRY_BEGIN_NAMESPACE | ||
namespace sdk | ||
{ | ||
namespace common | ||
{ | ||
/** | ||
Reads the container.id from /proc/self/cgroup file. | ||
@param file_path file path of cgroup | ||
@return container.id as string or empty string | ||
*/ | ||
std::string GetContainerIDFromCgroup(const char *file_path); | ||
|
||
/** | ||
Matches the line with the regex to find container.id | ||
@param line contains | ||
@return matched id or empty string | ||
*/ | ||
std::string ExtractContainerIDFromLine(std::string &line); | ||
} // namespace common | ||
} // namespace sdk | ||
OPENTELEMETRY_END_NAMESPACE | ||
nikhilbhatia08 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,53 @@ | ||||||||||||||||||||||||||||
// Copyright The OpenTelemetry Authors | ||||||||||||||||||||||||||||
// SPDX-License-Identifier: Apache-2.0 | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
#include "opentelemetry/sdk/common/container.h" | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
#ifdef _MSC_VER | ||||||||||||||||||||||||||||
# include <string.h> | ||||||||||||||||||||||||||||
# define strcasecmp _stricmp | ||||||||||||||||||||||||||||
#else | ||||||||||||||||||||||||||||
# include <strings.h> | ||||||||||||||||||||||||||||
#endif | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
#include <fstream> | ||||||||||||||||||||||||||||
#include <regex> | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
#include "opentelemetry/version.h" | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
OPENTELEMETRY_BEGIN_NAMESPACE | ||||||||||||||||||||||||||||
namespace sdk | ||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||
namespace common | ||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||
std::string GetContainerIDFromCgroup(const char* file_path) | ||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||
std::ifstream cgroup_file(file_path); | ||||||||||||||||||||||||||||
|
std::ifstream cgroup_file(file_path); | |
std::ifstream cgroup_file(file_path); | |
if (!cgroup_file.is_open()) | |
{ | |
return ""; | |
} |
Copilot uses AI. Check for mistakes.
Outdated
Copilot
AI
Jul 29, 2025
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.
Missing space after 'while'. Should be 'while (std::getline(cgroup_file, line))' to follow consistent formatting style.
while(std::getline(cgroup_file, line)) | |
while (std::getline(cgroup_file, line)) |
Copilot uses AI. Check for mistakes.
Outdated
Copilot
AI
Jul 29, 2025
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.
Missing space after 'if'. Should be 'if (!container_id.empty())' to follow consistent formatting style.
if(!container_id.empty()) | |
if (!container_id.empty()) |
Copilot uses AI. Check for mistakes.
nikhilbhatia08 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
Copilot
AI
Jul 30, 2025
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.
The regex pattern contains unexplained magic logic. Consider adding a comment explaining what container ID formats this regex is designed to match (e.g., Docker, containerd, cri-o) and provide examples of the expected input patterns.
{ | |
{ | |
// This regex is designed to extract container IDs from cgroup file lines. | |
// It matches hexadecimal container IDs used by container runtimes like Docker, containerd, and cri-o. | |
// The pattern breakdown: | |
// - ^.*/: Matches any path prefix up to the container ID. | |
// - (?:.*[-:])?: Optionally matches a prefix ending with '-' or ':' (e.g., "docker-", "containerd-"). | |
// - ([0-9a-f]+): Captures the container ID, which is a sequence of hexadecimal characters. | |
// - (?:\.|\s*$): Ensures the container ID is followed by a '.' or the end of the line. | |
// Examples of matching lines: | |
// - "/docker/1234567890abcdef" -> "1234567890abcdef" | |
// - "/kubepods/burstable/pod1234/containerd-abcdef1234567890" -> "abcdef1234567890" | |
// - "/kubepods/pod5678/cri-containerd-0987654321abcdef.scope" -> "0987654321abcdef" |
Copilot uses AI. Check for mistakes.
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -4,8 +4,10 @@ | |||||||||||||
#include "opentelemetry/sdk/resource/resource_detector.h" | ||||||||||||||
#include "opentelemetry/nostd/variant.h" | ||||||||||||||
#include "opentelemetry/sdk/common/env_variables.h" | ||||||||||||||
#include "opentelemetry/sdk/common/container.h" | ||||||||||||||
#include "opentelemetry/sdk/resource/resource.h" | ||||||||||||||
#include "opentelemetry/semconv/service_attributes.h" | ||||||||||||||
#include "opentelemetry/semconv/incubating/container_attributes.h" | ||||||||||||||
#include "opentelemetry/version.h" | ||||||||||||||
|
||||||||||||||
#include <stddef.h> | ||||||||||||||
|
@@ -21,6 +23,10 @@ namespace resource | |||||||||||||
|
||||||||||||||
const char *OTEL_RESOURCE_ATTRIBUTES = "OTEL_RESOURCE_ATTRIBUTES"; | ||||||||||||||
const char *OTEL_SERVICE_NAME = "OTEL_SERVICE_NAME"; | ||||||||||||||
/** | ||||||||||||||
* This is the file path from where we can get container.id | ||||||||||||||
*/ | ||||||||||||||
const char *C_GROUP_PATH = "/proc/self/cgroup"; | ||||||||||||||
nikhilbhatia08 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||
|
||||||||||||||
Resource ResourceDetector::Create(const ResourceAttributes &attributes, | ||||||||||||||
const std::string &schema_url) | ||||||||||||||
|
@@ -68,6 +74,23 @@ Resource OTELResourceDetector::Detect() noexcept | |||||||||||||
return ResourceDetector::Create(attributes); | ||||||||||||||
} | ||||||||||||||
|
||||||||||||||
Resource ContainerResourceDetector::Detect() noexcept | ||||||||||||||
nikhilbhatia08 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||||||
{ | ||||||||||||||
std::string container_id = opentelemetry::sdk::common::GetContainerIDFromCgroup(C_GROUP_PATH); | ||||||||||||||
if(container_id.empty()) | ||||||||||||||
|
if(container_id.empty()) | |
if (container_id.empty()) |
Copilot uses AI. Check for mistakes.
Outdated
Copilot
AI
Jul 29, 2025
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.
Missing space after 'if'. Should be 'if (!container_id.empty())' to follow consistent formatting style.
if(!container_id.empty()) | |
if (!container_id.empty()) |
Copilot uses AI. Check for mistakes.
Outdated
Copilot
AI
Jul 29, 2025
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.
This condition is redundant since container_id.empty() was already checked at line 80. If container_id is empty, the function returns early, so this check is unnecessary.
if(!container_id.empty()) | |
{ | |
attributes[semconv::container::kContainerId] = container_id; | |
} | |
attributes[semconv::container::kContainerId] = container_id; |
Copilot uses AI. Check for mistakes.
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -8,9 +8,11 @@ | |||||||||||||||||||||||||||||||||||||||||
#include <string> | ||||||||||||||||||||||||||||||||||||||||||
#include <unordered_map> | ||||||||||||||||||||||||||||||||||||||||||
#include <utility> | ||||||||||||||||||||||||||||||||||||||||||
#include<fstream> | ||||||||||||||||||||||||||||||||||||||||||
|
#include<fstream> | |
#include <fstream> |
Copilot uses AI. Check for mistakes.
Outdated
Copilot
AI
Jul 29, 2025
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.
Typo in test name: 'ExctractValidContainerId' should be 'ExtractValidContainerId'.
TEST(ResourceTest, ExctractValidContainerId) | |
TEST(ResourceTest, ExtractValidContainerId) |
Copilot uses AI. Check for mistakes.
Outdated
Copilot
AI
Jul 30, 2025
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.
The test file cleanup should be in a finally block or use RAII to ensure cleanup occurs even if the test fails. Consider using a test fixture with proper cleanup or a temporary file wrapper.
std::remove(filename); |
Copilot uses AI. Check for mistakes.
Outdated
Copilot
AI
Jul 30, 2025
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.
The test file cleanup should be in a finally block or use RAII to ensure cleanup occurs even if the test fails. Consider using a test fixture with proper cleanup or a temporary file wrapper.
const char *filename = "test_empty_cgroup.txt"; | |
{ | |
std::ofstream outfile(filename); | |
outfile << "no container id here\n"; | |
} | |
std::string id = opentelemetry::sdk::common::GetContainerIDFromCgroup(filename); | |
EXPECT_EQ(id, std::string{""}); | |
std::remove(filename); // cleanup | |
TemporaryFile temp_file("test_empty_cgroup.txt"); | |
{ | |
std::ofstream outfile(temp_file.GetFilename()); | |
outfile << "no container id here\n"; | |
} | |
std::string id = opentelemetry::sdk::common::GetContainerIDFromCgroup(temp_file.GetFilename()); | |
EXPECT_EQ(id, std::string{""}); |
Copilot uses AI. Check for mistakes.
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.
Incomplete documentation comment. The '@param line contains' description is cut off and should specify what the line parameter contains.
Copilot uses AI. Check for mistakes.