Skip to content

Commit

Permalink
fix(otel): Monitored Resource with task_id for Cloud Run
Browse files Browse the repository at this point in the history
Fixes #14925

When inside a Cloud Run environment, the `MonitoredResource` in a `CreateTimeSeriesRequest` to the Cloud Monitoring API does not include the necessary fields for the `generic_task` resource type, and is rejected.

Should follow the well-tested Golang implementation where the `faas.instance` OTel Resource Attribute is mapped to `MonitoredResource` `task_id`. As the `service.namespace` OTel Resource Attribute is not set by the Resource Detector from within Cloud Run, it should be mapped as an empty string, rather than being left absent.

https://github.com/GoogleCloudPlatform/opentelemetry-operations-go/blob/8da0f42dab085c916987891419461d583a2aa96e/internal/resourcemapping/resourcemapping.go#L153
  • Loading branch information
DouglasHeriot committed Jan 15, 2025
1 parent 51e05fe commit 7a0f137
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 11 deletions.
12 changes: 6 additions & 6 deletions google/cloud/opentelemetry/internal/monitored_resource.cc
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ struct AsStringVisitor {

struct OTelKeyMatch {
std::vector<std::string> otel_keys;
absl::optional<std::string> fallback = absl::nullopt;
std::string fallback;
};

class MonitoredResourceProvider {
Expand All @@ -72,8 +72,8 @@ class MonitoredResourceProvider {
[](auto const& key, auto const& attr) { return key == attr.first; });
if (found != oks.end()) {
mr.labels[kv.first] = AsString(attributes.at(*found));
} else if (kv.second.fallback) {
mr.labels[kv.first] = *kv.second.fallback;
} else {
mr.labels[kv.first] = kv.second.fallback;
}
}
return mr;
Expand Down Expand Up @@ -162,8 +162,8 @@ MonitoredResourceProvider GenericTask() {
{"location",
{{sc::kCloudAvailabilityZone, sc::kCloudRegion}, "global"}},
{"namespace", {{sc::kServiceNamespace}}},
{"job", {{sc::kServiceName}}},
{"task_id", {{sc::kServiceInstanceId}}},
{"job", {{sc::kServiceName, sc::kFaasName}}},
{"task_id", {{sc::kServiceInstanceId, sc::kFaasInstance}}},
});
}

Expand All @@ -174,7 +174,7 @@ MonitoredResourceProvider GenericNode() {
{"location",
{{sc::kCloudAvailabilityZone, sc::kCloudRegion}, "global"}},
{"namespace", {{sc::kServiceNamespace}}},
{"node_id", {{sc::kHostId}}},
{"node_id", {{sc::kHostId, sc::kHostName}}},
});
}

Expand Down
39 changes: 34 additions & 5 deletions google/cloud/opentelemetry/internal/monitored_resource_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,11 @@ TEST(MonitoredResource, GenericTaskFaas) {
auto mr = ToMonitoredResource(attributes);
EXPECT_EQ(mr.type, "generic_task");
EXPECT_THAT(mr.labels,
UnorderedElementsAre(Pair("location", test.expected_location)));
UnorderedElementsAre(Pair("location", test.expected_location),
// Verify fallback to empty string.
Pair("namespace", ""),
Pair("job", "faas-name"),
Pair("task_id", "faas-instance")));
}
}

Expand Down Expand Up @@ -320,18 +324,18 @@ TEST(MonitoredResource, GenericTaskService) {
}

TEST(MonitoredResource, GenericNode) {
struct TestCase {
struct LocationTestCase {
absl::optional<std::string> zone;
absl::optional<std::string> region;
std::string expected_location;
};
auto tests = std::vector<TestCase>{
auto location_tests = std::vector<LocationTestCase>{
{"us-central1-a", "us-central1", "us-central1-a"},
{"us-central1-a", absl::nullopt, "us-central1-a"},
{absl::nullopt, "us-central1", "us-central1"},
{absl::nullopt, absl::nullopt, "global"},
};
for (auto const& test : tests) {
};
for (auto const& test : location_tests) {
auto attributes = opentelemetry::sdk::resource::ResourceAttributes{
{sc::kServiceNamespace, "test-namespace"},
{sc::kHostId, "test-instance"},
Expand All @@ -346,6 +350,31 @@ TEST(MonitoredResource, GenericNode) {
Pair("namespace", "test-namespace"),
Pair("node_id", "test-instance")));
}

struct NodeIDTestCase {
absl::optional<std::string> host_id;
std::string expected_node_id;
};
auto node_id_tests = std::vector<NodeIDTestCase>{
{"test-instance", "test-instance"},
{absl::nullopt, "test-name"},
};
for (auto const& test : node_id_tests) {
auto attributes = opentelemetry::sdk::resource::ResourceAttributes{
{sc::kCloudAvailabilityZone, "us-central1-a"},
{sc::kCloudRegion, "us-central1"},
{sc::kServiceNamespace, "test-namespace"},
{sc::kHostName, "test-name"},
};
if (test.host_id) attributes[sc::kHostId] = *test.host_id;

auto mr = ToMonitoredResource(attributes);
EXPECT_EQ(mr.type, "generic_node");
EXPECT_THAT(mr.labels,
UnorderedElementsAre(Pair("location", "us-central1-a"),
Pair("namespace", "test-namespace"),
Pair("node_id", test.expected_node_id)));
}
}

} // namespace
Expand Down

0 comments on commit 7a0f137

Please sign in to comment.