Skip to content

Commit

Permalink
fix(otel): Exporter creating Monitored Resource with task_id for Clou…
Browse files Browse the repository at this point in the history
…d Run (#14923)

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 authored Jan 16, 2025
1 parent a8c2e9d commit 63bc533
Show file tree
Hide file tree
Showing 2 changed files with 40 additions and 11 deletions.
13 changes: 7 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,8 @@ struct AsStringVisitor {

struct OTelKeyMatch {
std::vector<std::string> otel_keys;
absl::optional<std::string> fallback = absl::nullopt;
// NOLINTNEXTLINE(readability-redundant-member-init)
std::string fallback = {};
};

class MonitoredResourceProvider {
Expand All @@ -72,8 +73,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 +163,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 +175,7 @@ MonitoredResourceProvider GenericNode() {
{"location",
{{sc::kCloudAvailabilityZone, sc::kCloudRegion}, "global"}},
{"namespace", {{sc::kServiceNamespace}}},
{"node_id", {{sc::kHostId}}},
{"node_id", {{sc::kHostId, sc::kHostName}}},
});
}

Expand Down
38 changes: 33 additions & 5 deletions google/cloud/opentelemetry/internal/monitored_resource_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -283,8 +283,11 @@ TEST(MonitoredResource, GenericTaskFaas) {

auto mr = ToMonitoredResource(attributes);
EXPECT_EQ(mr.type, "generic_task");
EXPECT_THAT(mr.labels,
UnorderedElementsAre(Pair("location", test.expected_location)));
EXPECT_THAT(mr.labels, 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 +323,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 +349,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 63bc533

Please sign in to comment.