-
Notifications
You must be signed in to change notification settings - Fork 83
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
added experimental package spans
with abstract layer for tracing
#1509
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1509 +/- ##
==========================================
- Coverage 70.95% 70.80% -0.16%
==========================================
Files 355 356 +1
Lines 37179 37180 +1
==========================================
- Hits 26382 26325 -57
- Misses 9701 9746 +45
- Partials 1096 1109 +13
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
github.com/ydb-platform/ydb-go-sdk/v3/logincompatible changes(*defaultLogger).Log: changed from func(context.Context, string, ...Field) to func(context.Context, string, ...github.com/ydb-platform/ydb-go-sdk/v3/internal/kv.KeyValue) github.com/ydb-platform/ydb-go-sdk/v3/spanscompatible changespackage added github.com/ydb-platform/ydb-go-sdk/v3/traceincompatible changesDriverOnBalancerClusterDiscoveryAttempt: changed from func(*Driver, *context.Context, call, string) func(error) to func(*Driver, *context.Context, call, string, string) func(error) compatible changesDriverBalancerClusterDiscoveryAttemptStartInfo.Database: added summaryBase version: v3.84.2-0.20241017174807-1bcbc910b99e (master) |
🌋 Here are results of SLO test for Go SDK xorm: |
🌋 Here are results of SLO test for Go SDK database/sql: |
🌋 Here are results of SLO test for Go SDK gorm: |
🌋 Here are results of SLO test for Native ydb-go-sdk/v3 over query-service: |
🌋 Here are results of SLO test for Native ydb-go-sdk/v3 over table-service: |
otel/discovery.go
Outdated
cfg, | ||
info.Context, | ||
info.Call.FunctionID(), | ||
kv.String("address", info.Address), |
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.
Есть соглашение о наименовании https://opentelemetry.io/docs/specs/semconv/. Можно глянуть на примеры ключей и как они формируются.
otel/discovery.go
Outdated
kv.String("address", info.Address), | ||
kv.String("database", info.Database), |
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.
kv.String("address", info.Address), | |
kv.String("database", info.Database), | |
kv.String("ydb.address", info.Address), | |
kv.String("ydb.database", info.Database), |
otel/retry.go
Outdated
cfg, | ||
info.Context, | ||
operationName, | ||
kv.Bool("idempotent", info.Idempotent), |
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.
kv.Bool("idempotent", info.Idempotent), | |
kv.Bool("ydb.retry.idempotent", info.Idempotent), |
otel/retry.go
Outdated
|
||
return func(info trace.RetryLoopDoneInfo) { | ||
fields := []KeyValue{ | ||
kv.Int("attempts", info.Attempts), |
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.
Как и раньше, в атрибуты лучше добавлять существенную информацию для фильтрации, быстрого поиска. А attempts - это можно в метрики вынести, чтобы следить за кол-во попыток и словить аномальное поведение.
otel/scripting.go
Outdated
cfg, | ||
info.Context, | ||
info.Call.FunctionID(), | ||
kv.String("query", info.Query), |
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.
kv.String("query", info.Query), | |
kv.String("ydb.query", info.Query), |
spans
with abstract layer for tracing
spans
with abstract layer for tracingspans
with abstract layer for tracing
Pull request type
Please check the type of change your PR introduces:
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Other information