Skip to content

Add open telemetry logging to Ibex Client - #59

Merged
brh28 merged 3 commits into
lnflash:mainfrom
brh28:ibex-otel
Apr 22, 2024
Merged

Add open telemetry logging to Ibex Client#59
brh28 merged 3 commits into
lnflash:mainfrom
brh28:ibex-otel

Conversation

@brh28

@brh28 brh28 commented Apr 12, 2024

Copy link
Copy Markdown
Contributor

Adds telemetry data to the Ibex client. This is done by wrapping the client with a call to wrapAsyncFunctionsToRunInSpan which gets the current span and adds attributes and exceptions to it.

Here's a sample trace in Honeycomb which shows an API call failing because the ibex accessToken & refreshToken are not present:

Screenshot from 2024-04-16 14-08-46

@brh28
brh28 marked this pull request as ready for review April 16, 2024 20:05
@brh28 brh28 linked an issue Apr 19, 2024 that may be closed by this pull request
Comment thread src/services/ibex/client/index.ts Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

any reason this is a function and not just an object?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was trying to avoid doing this as well, but couldn't figure out to de-structure the class into the wrapAsyncFunctionsToRunInSpan function. If I remember correctly, I also tried to define it as an object, but couldn't figure that out either, so instead, I settled for this approach, which is similar to the how the rest of the code base is defined.

IMO, this would be good to revisit at some point. But for now, I'd rather merge what works than spend more time trying to perfect

@abh3po

abh3po commented Apr 20, 2024

Copy link
Copy Markdown
Contributor

overall looks good but changing the AuthenticatedIbexClient class to a function instead of an object seems unnecessarily complex, unless there''s a reason that I don't understand.

@brh28
brh28 merged commit d624940 into lnflash:main Apr 22, 2024
@brh28
brh28 deleted the ibex-otel branch May 30, 2024 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add telemetry data to the Ibex client

2 participants