Add OpenTelemetry lib and metric-interfaces#588
Add OpenTelemetry lib and metric-interfaces#588maladetska wants to merge 3 commits intoydb-platform:mainfrom
Conversation
| void ParseEndpoint(const std::string& endpoint, std::string& host, int& port) { | ||
| auto pos = endpoint.find(':'); | ||
| if (pos != std::string::npos) { | ||
| host = endpoint.substr(0, pos); | ||
| try { | ||
| port = std::stoi(endpoint.substr(pos + 1)); | ||
| } catch (...) { | ||
| port = 2135; | ||
| } | ||
| } else { | ||
| host = endpoint; | ||
| port = 2135; | ||
| } | ||
| } |
There was a problem hiding this comment.
это плохо, пока черновой вариант такой
There was a problem hiding this comment.
может быть, на файлы разбить?
maladetska
force-pushed
the
METRICS-0
branch
from
beec910 to
a102d53
Compare
🌋 SLO Test ResultsStatus: 🟢 2 workloads tested • All passed
Generated by ydb-slo-action |
| find_package(opentelemetry-cpp QUIET) | ||
| if (opentelemetry-cpp_FOUND) | ||
| set(YDB_SDK_HAS_OPEN_TELEMETRY ON) | ||
| endif() |
There was a problem hiding this comment.
opentelemetry -- опциональная зависимость
| add_subdirectory(util) | ||
|
|
||
| if (YDB_SDK_HAS_OPEN_TELEMETRY) | ||
| add_subdirectory(open_telemetry EXCLUDE_FROM_ALL) |
There was a problem hiding this comment.
не будет собираться если на него не сошлются
maladetska
changed the title
Gazizonoki
left a comment
Gazizonoki
left a comment
There was a problem hiding this comment.
Мне кажется папка open_telemetry в корне не самая удобная штука. Это же плагин к sdk, может сделаем папку plugins(extensions или еще какое-то имя) и туда положим все такие штуки (экспортеры метрик, трейсов, логов для конкретных систем и т. д.)?
И с метриками, оно вообще используется по коду? SDK уже собирает много метрик, в задаче с метриками это и есть главная сложность, как аккуратно перевести все на общий интерфейс и выкинуть library/cpp/monlib уже к черту из внешнего репозитория (это так-то внутренняя штука, вылезла в open-source ydb-cpp-sdk из-за неаккуратности)
There was a problem hiding this comment.
Как-то намешано все тут. И про метрики, и про спаны
| virtual std::shared_ptr<ITracer> GetTracer(const std::string& name) = 0; | ||
| }; | ||
|
|
||
| class IMetricsApi : public IExtensionApi { |
There was a problem hiding this comment.
IExtensionApi - большое зло. Не очень удачное решение, которое плотно так засело в SDK. Давайте закопаем его. Мне кажется, пользователю будет гораздо удобнее, если экспортер метрик и трейсов будут прям в TDriverConfig задаваться.
Типа
NYdb::TDriver driver(TDriverConfig("grpc://localhost:2136/local")
.SetMetricExporter(CreateOtelMetricExporter(...))
.SetTraceExporter(CreateOtelTraceExporter(...))
);
...Лаконично и понятно пользователю.
There was a problem hiding this comment.
А может разделим? Тащить и метрики и трейсы сразу как-то странно, все же разные сущности. Вдруг трейсы пользователю не нужны?
|
|
||
| TOtelMetricRegistry::TOtelMetricRegistry(nostd::shared_ptr<metrics::MeterProvider> meterProvider) | ||
| : MeterProvider_(std::move(meterProvider)) | ||
| , Meter_(MeterProvider_->GetMeter("ydb-cpp-sdk", "1.0.0")) |
There was a problem hiding this comment.
Почему 1.0.0? У ydb-cpp-sdk есть версия, в SLO тестах даже проставляем на meter: https://github.com/ydb-platform/ydb-cpp-sdk/blob/main/tests/slo_workloads/utils/metrics.cpp#L53
| return std::make_shared<TOtelUpDownCounterGauge>(std::move(counter), labels); | ||
| } | ||
|
|
||
| std::shared_ptr<IHistogram> TOtelMetricRegistry::Histogram(const std::string& name, const std::vector<double>& buckets, const TLabels& labels) { |
| } // namespace | ||
|
|
||
| TQuerySpan::TQuerySpan(std::shared_ptr<NMetrics::ITracer> tracer, const std::string& operationName, const std::string& endpoint) { | ||
| if (!tracer) return; |
There was a problem hiding this comment.
| if (!tracer) return; | |
| if (!tracer) { | |
| return; | |
| } |
|
|
||
| TQuerySpan::~TQuerySpan() { | ||
| if (Span_) { | ||
| Span_->End(); |
There was a problem hiding this comment.
Это же поход по сети? А если ошибка, то исключение вылетит?
|
|
||
| void ReplyError(TStatus status) override { | ||
| TSession session; | ||
| if (Span) Span->End(status.GetStatus()); |
There was a problem hiding this comment.
| if (Span) Span->End(status.GetStatus()); | |
| if (Span) { | |
| Span->End(status.GetStatus()); | |
| } |
| ) | ||
| ); | ||
|
|
||
| if (Span) Span->End(EStatus::SUCCESS); |
| { | ||
| promise.SetValue(future.ExtractValue()); | ||
| auto val = future.ExtractValue(); | ||
| if (span) span->End(val.GetStatus()); |
2 participants
No description provided.