-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: use opentelemetry library for tracing #60
Conversation
It will be reviewed after #61 |
8afdaa1
to
23be439
Compare
logFunction: (extra: Dict | undefined, message: string) => void, | ||
): DiagLogFunction { | ||
return (message, ...args) => { | ||
const data = args.reduce((acc: Record<string, any>, cur, index) => { |
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.
const data = args.reduce((acc: Record<string, any>, cur, index) => { | |
const data = args.reduce((acc: Record<string, unknown>, cur, index) => { |
return (message, ...args) => { | ||
const data = args.reduce((acc: Record<string, any>, cur, index) => { | ||
if (typeof cur === 'object' && !Array.isArray(cur)) { | ||
acc = { |
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.
It would be nice to add
"no-param-reassign": [
"warn",
{
"props": true,
"ignorePropertyModificationsFor": ["acc"]
}
]
to the .eslintrc
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.
I refactored the code and now it's without warnings
src/lib/tracing/init-tracing.ts
Outdated
traceExporter: | ||
appTracingSpanExporter || | ||
new OTLPTraceExporter({ | ||
url: appTracingCollectorEndpoint || DEFAULT_COLLECTOR_HOST, |
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.
I think DEFAULT_COLLECTOR_HOST is redundant.
url is optional and can be omitted - default is http://localhost:4318/v1/traces
https://www.npmjs.com/package/@opentelemetry/exporter-trace-otlp-http
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.
agree
@melikhov-dev I updated the pull request, take a look |
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.
LGTM
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en.
close #2