-
Notifications
You must be signed in to change notification settings - Fork 66
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
chore: Datadog PoC [OTE-702] [2/n] #932
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
71acfe7
to
fb208b2
Compare
6ba0b21
to
b0db1e7
Compare
fb208b2
to
d82cfb5
Compare
src/lib/abacus/logger.ts
Outdated
@@ -19,6 +21,20 @@ class AbacusLogger implements Omit<AbacusLoggingProtocol, '__doNotUseOrImplement | |||
console.error(`${tag}: ${message}`); | |||
} | |||
} | |||
|
|||
ddInfo(message: string, context: object) { |
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.
@dydxprotocol/frontend this is how im planning on giving abacus the power to log info and error msgs to datadog.
it's a little hacky but until we're ready to merge their logs i figure this is good enough.
src/lib/analytics/amplitude.ts
Outdated
@@ -22,7 +22,7 @@ export const identify = (property: AnalyticsUserProperty) => { | |||
detail: { property: propertyTypeToLog, propertyValue: property.payload }, | |||
}); | |||
dd.setContextProperty(propertyTypeToLog, property.payload); | |||
dd.log(`set context item: ${propertyTypeToLog}`, dd.getContext()); | |||
dd.info(`set context item: ${propertyTypeToLog}`, dd.getContext()); |
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.
Does it make sense to just add dd.info
to track
? or was it decided that is too much events
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.
this was discussed in slack but we decided to be very conservative about the # of events we send and track the volume first
a053085
to
5ca2cf7
Compare
50262b2
to
a2a3c4d
Compare
25d31ca
to
e0b5ffd
Compare
e0b5ffd
to
916fd06
Compare
add dd file revert usecompliancestate hook fix import names fix last broken import filepath add events to deposit and withdraw forms fix import names dydx chain transactions logs. add abacus logger implementation. auto log errors finish logger implementation add service name and env fix logging fix type typo
3ff9137
to
a21f598
Compare
4c79cb9
to
6e2a0d5
Compare
src/lib/abacus/logger.ts
Outdated
dd.info(message, parsedContext); | ||
// catch in case parsing context fails for some reason | ||
} catch (err) { | ||
dd.error('Error sending dd info', undefined, err); |
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.
maybe send the context string in this case?
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.
ah good point, forgot the error won't have that data. i like it
Adds initial events described by the tech spec
Also adds ddInfo and ddError methods to abacus logger implementation so we can pick them up in abacus and use them at will.
Finally, this PR adds
dd.error
to thelog
method, so any telemetry logs will automatically be piped into datadog