opentelemetry-cpp-contrib icon indicating copy to clipboard operation
opentelemetry-cpp-contrib copied to clipboard

Offer a GCP resource detector

Open dbolduc opened this issue 2 years ago • 4 comments

Objective

  • Make it simple for customers to tie telemetry to the GCP resource producing it.
  • Get one step closer to a universal OOTB resource detector

Background

  • GCP resource detection is a function of the environment variables and the response (if any) from the metadata server
  • The call we are making to the metadata server returns json.
  • A C++ GCP resource detector exists in googleapis/google-cloud-cpp.
    • The existing resource detector depends on google-cloud-cpp internals
    • This is convenient for maintainers of said library, but not worth the cost of an extra dependency for customers.

Detailed Design

Location

I would make new directories: opentelemetry-cpp/detectors/gcp/...

$ tree detectors/gcp

detectors/gcp
├── BUILD
├── CMakeLists.txt
├── include
│   └── opentelemetry
│       └── detectors
│           └── gcp
│               ├── internal
│               │   └── resource_detector_impl.h
│               └── resource_detector.h
├── README.md
├── src
│   ├── resource_detector.cc
│   └── resource_detector_impl.cc
└── test
    └── resource_detector_test.cc

Dependencies

  • nlohmann/json
  • libcurl
    • Well, it will use the opentelemetry::ext::http::HttpClientSync. I think that forces the dep on libcurl, but I am not sure.

Interface

namespace opentelemetry::detectors::gcp
{

// Configuration options for the GCP Resource Detector
struct GcpDetectorOptions
{
  std::string endpoint = "http://metadata.google.internal";
  // ... and more?
};

// Make a GCP Resource Detector.
std::unique_ptr<sdk::resource::ResourceDetector> MakeGcpDetector(GcpDetectorOptions options = {});

} // namespace opentelemetry::detectors::gcp

// The internal namespace is for implementation details only. The symbols within are not part of the
// public API. They are subject to change, including deletion, without notice.
namespace opentelemetry::detectors::gcp::internal
{

// Interface to simplify testing. The default will sleep.
class Retry
{
  // Returns `true` if we should keep retrying, `false` if we should stop retrying.
  virtual bool OnRetry() = 0;
};

// In tests, we mock the client and the retry policy.
std::unique_ptr<sdk::resource::ResourceDetector> MakeGcpDetector(
    std::shared_ptr<ext::http::client::HttpClientSync> client,
    std::shared_ptr<Retry> retry,
    GcpDetectorOptions options = {});

}  // namespace opentelemetry::detectors::gcp::internal

Testing

  • Unit tests will be a thing.
  • Adding end-to-end integration tests for all platforms is infeasible.
    • Note that E2E testing is done on GCP resource detectors in other languages.
    • We will just hope that C++ ~~plagiarizes~~ copies those languages' code correctly.

dbolduc avatar Jul 13 '23 22:07 dbolduc

To the opentelemetry-cpp maintainers:

I intend to do this work. I have a prototype. Of course it will take some effort to make the code not suck.

If offering a GCP resource detector in this repo is not a good idea, please let me know. If the proposal is ok, please assign the issue to me and I will keep working on it.

dbolduc avatar Jul 13 '23 22:07 dbolduc

This would normally goes to contrib repo as the main repo is only for hosting api, sdk and exporters that are in the OpenTelemetry spec. But if there was similar implementation accepted by the other SIGs in their main repo, we will also accept to have it in the main repo.

esigo avatar Jul 17 '23 20:07 esigo

I looked into where other SIGs keep resource detectors:

Project Location type Location
Collector Contrib https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/processor/resourcedetectionprocessor/internal
Go Contrib https://github.com/open-telemetry/opentelemetry-go-contrib/tree/main/detectors
Java Contrib https://github.com/open-telemetry/opentelemetry-java-contrib/tree/main/aws-resources
Java Out-of-tree https://github.com/GoogleCloudPlatform/opentelemetry-operations-java/tree/main/detectors/resources
JS Contrib https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/detectors/node
Python Contrib https://github.com/open-telemetry/opentelemetry-python-contrib/tree/main/sdk-extension/opentelemetry-sdk-extension-aws/src/opentelemetry/sdk/extension/aws/resource
Python Out-of-tree https://github.com/GoogleCloudPlatform/opentelemetry-operations-python/tree/main/opentelemetry-resourcedetector-gcp

It doesn't look like anybody else keeps them in the core repo.

punya avatar Jul 25 '23 14:07 punya

@dbolduc I moved the issue to the contribe repo, please feel free to raise PR here.

esigo avatar Jul 29 '23 10:07 esigo