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/track metrics #1019

Merged
merged 36 commits into from
Dec 13, 2023
Merged

Feat/track metrics #1019

merged 36 commits into from
Dec 13, 2023

Conversation

elraphty
Copy link
Contributor

Describe your changes

This PR adds the metrics functionality

Issue ticket number and link

Closes #768

Type of change

  • New feature (non-breaking change which adds functionality)
  • This change requires a documentation update

Checklist before requesting a review

  • I have performed a self-review of my code
  • I have tested on Chrome and Firefox
  • I have tested on a mobile device

@elraphty elraphty requested a review from kevkevinpal November 28, 2023 18:40
Comment on lines +17 to +24
redisURL := os.Getenv("REDIS_URL")

if redisURL == "" {
dbInt, _ := utils.ConvertStringToInt(os.Getenv("REDIS_DB"))
RedisClient = redis.NewClient(&redis.Options{
Addr: os.Getenv("REDIS_HOST"),
Username: os.Getenv("REDIS_USER"),
Password: os.Getenv("REDIS_PASS"),
Copy link
Contributor

Choose a reason for hiding this comment

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

these new env vars we might want to add to the README.md or somewhere similar

routes/metrics.go Outdated Show resolved Hide resolved
routes/metrics.go Outdated Show resolved Hide resolved
Comment on lines +140 to +142
Price, ok5 := migrateBounty["price"].(uint)
if !ok5 {
migrateBountyFinal.Price = "0"
migrateBountyFinal.Price = 0
Copy link
Contributor

Choose a reason for hiding this comment

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

so I think this only happened when we did that initial migration so I don't think this will modify any existing bounties so they will continue to be strings. Not sure if we should do this change, we should honestly remove this migration code aswell since if we run it again we will have duplicate bounties which is not good

Comment on lines +189 to +191
if (newBody.price) {
newBody.price = Number(newBody.price);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This fixes the issue of the bounty price being a string right? not sure if we want to return as a number from the backend so the frontend doesn't have to do this. But that may be a larger change

db/structs.go Outdated Show resolved Hide resolved
db/structs.go Outdated Show resolved Hide resolved
Comment on lines +646 to +653
BountiesPosted int64 `json:"bounties_posted"`
BountiesPaid int64 `json:"bounties_paid"`
BountiesPaidPercentage uint `json:"bounties_paid_average"`
SatsPosted uint `json:"sats_posted"`
SatsPaid uint `json:"sats_paid"`
SatsPaidPercentage uint `json:"sats_paid_percentage"`
AveragePaid uint `json:"average_paid"`
AverageCompleted uint `json:"average_completed"`
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a reason why two of them are int64 and the rest are uint

db/redis.go Outdated
}

func SetValue(key string, value interface{}) {
err := RedisClient.Set(ctx, key, value, 6*time.Hour).Err()
Copy link
Contributor

Choose a reason for hiding this comment

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

can we make 6*time.Hour a var so it is more obvious incase we want to change that value in the future?

"github.com/stakwork/sphinx-tribes/utils"
)

func (db database) TotalPeopleByDateRange(r PaymentDateRange) int64 {
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont see this being used anywhere

db/metrics.go Outdated
return 0
}

func (db database) CompletedDifferenceSum(r PaymentDateRange) uint {
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont see this being used anywhere

db/metrics.go Outdated
return sum
}

func (db database) CompletesDifferenceCount(r PaymentDateRange) int64 {
Copy link
Contributor

Choose a reason for hiding this comment

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

I dont see this being used anywhere

config/config.go Outdated
@@ -29,6 +30,9 @@ func InitConfig() {
RelayUrl = os.Getenv("RELAY_URL")
MemeUrl = os.Getenv("MEME_URL")
RelayAuthKey = os.Getenv("RELAY_AUTH_KEY")
AdminStrings := os.Getenv("SUPER_ADMINS")
Copy link
Contributor

Choose a reason for hiding this comment

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

If you can add this ENV var to the README and what it is used for that would be helpful

Copy link
Contributor

Choose a reason for hiding this comment

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

@elraphty can you change this value to just ADMINS aswell SUPER_ADMINS seems a bit much especially since we don't even have the concept of admins implemented yet

@elraphty elraphty merged commit ab480c0 into master Dec 13, 2023
6 checks passed
@ecurrencyhodler
Copy link
Contributor

Great job Raph. This was a big one that unblocks a lot of work.

@elraphty did we open up endpoints? If so which ones?

Here is the issue related to that as well: #1007

elraphty added a commit that referenced this pull request Jan 26, 2024
@Evanfeenstra Evanfeenstra deleted the feat/track_metrics branch January 26, 2024 17:56
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.

Track metrics in the bounties platform (Postgres)
3 participants