Skip to content
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: return btc price in stats endpoint #167

Merged
merged 19 commits into from
Dec 4, 2024

Conversation

gusin13
Copy link
Contributor

@gusin13 gusin13 commented Dec 3, 2024

Fixes - #166

This pr

  1. Adds a new optional config external_apis please ref to config_local.yml
  2. If the config is provided it will fetch btc price from coin market cap
  3. If the config not provided it will not fetch any data, the btc price will be omitted from /v1/stats endpoint
  4. Mongo db TTL can be set in config, currently set to 3seconds, after this the document will be auto deleted
  5. The api key need to be provided in config as well cc @filippos47
  6. Doesn't contain tests (needs some more work to setup mocks etc)

When fetched first time
Screenshot 2024-12-03 at 5 39 17 PM

After TTL (currently 3 seconds in local config)
Screenshot 2024-12-03 at 5 39 31 PM

in /v1/stats

Screenshot 2024-12-03 at 5 53 02 PM

@gusin13 gusin13 marked this pull request as ready for review December 3, 2024 18:23
Copy link
Contributor

@jeremy-babylonlabs jeremy-babylonlabs left a comment

Choose a reason for hiding this comment

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

lgtm, amazing work

price, err := s.GetLatestBtcPriceUsd(ctx)
if err != nil {
log.Ctx(ctx).Error().Err(err).Msg("error while fetching latest btc price")
return nil, types.NewInternalServiceError(err)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's not fail the whole request due to this. We should emit a metric and have alert based on this, but silently fail by fall back to empty value for the additional field we added in the stats response.
FE will handle it gracefully.

@gusin13 gusin13 merged commit d45c0c0 into v0.3.x Dec 4, 2024
11 checks passed
@jrwbabylonlab jrwbabylonlab deleted the 166-return-current-btc-price-in-v1stats branch December 4, 2024 10:44
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.

3 participants