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

added screen that shows transaction graph per hour, day and month #92

Merged
merged 5 commits into from
Jun 7, 2023

Conversation

woutslakhorst
Copy link
Member

part of #36

this PR adds a page with three graphs showing transaction counts.

Next step is to show counts per type of transactions.
Then show a top 10 of DID controllers with the most transactions total.
Then connect Nats

Screenshot 2023-05-25 at 19 20 27

@woutslakhorst
Copy link
Member Author

I'm positioning this as a cheap stand-alone solution. A different approach would be exporting it as metric to a hosted tool like Datadog. That would involve more setup and would not make it available for everyone.

*
*/

package data
Copy link
Member

Choose a reason for hiding this comment

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

Intellectual exercise, but weren't there libraries we could simply drop in? (Prometheus-esque?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, but your deployment will also require 2 additional containers (prometheus and grafana), this requires additional devops work and costs for hosting.

This app is currently only for the foundation to track production network health. This is the quick and dirty solution for visualising something.

ctx, cancel := context.WithCancel(context.Background())
defer cancel()
store := data.NewStore()
loadHistory(store, config)
Copy link
Member

Choose a reason for hiding this comment

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

app has to be restarted to show new data?

Copy link
Member Author

Choose a reason for hiding this comment

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

As shown in the PR description, wip: Then connect Nats

main.go Outdated Show resolved Hide resolved
@reinkrul
Copy link
Member

reinkrul commented Jun 1, 2023

I think using something like Prometheus will support future use cases better, e.g. it allows you to add labels (which can be a TX type, for example).

data/window.go Outdated Show resolved Hide resolved
data/window.go Outdated Show resolved Hide resolved
data/window.go Outdated Show resolved Hide resolved
data/window.go Outdated Show resolved Hide resolved
data/window.go Show resolved Hide resolved
data/window_test.go Show resolved Hide resolved
data/store.go Outdated Show resolved Hide resolved
var transactions [3][]DataPoint

for i, window := range s.slidingWindows {
transactions[i] = window.dataPoints
Copy link
Member

Choose a reason for hiding this comment

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

access not protected by the window.mutex, not sure if it matters here

Copy link
Member Author

Choose a reason for hiding this comment

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

yup, ran it with -race and it failed

Copy link
Member

Choose a reason for hiding this comment

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

still not protected?

Copy link
Member Author

Choose a reason for hiding this comment

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

Mutex has been placed on Public methods. This does not totally block things for the store (same package), it's more a convention. Otherwise you get lock-spaghetti or window needs its own package (overkill)

api/api.yaml Outdated Show resolved Hide resolved
api/api.go Outdated Show resolved Hide resolved
@woutslakhorst
Copy link
Member Author

I think using something like Prometheus will support future use cases better, e.g. it allows you to add labels (which can be a TX type, for example).

There shouldn't be future use-cases.

  • VC TXs are going to OID4C
  • DID updates are going to did:web probably
  • Assertions will feature a different mechanism probably

Co-authored-by: Gerard Snaauw <[email protected]>
@reinkrul
Copy link
Member

reinkrul commented Jun 2, 2023

I think using something like Prometheus will support future use cases better, e.g. it allows you to add labels (which can be a TX type, for example).

There shouldn't be future use-cases.

  • VC TXs are going to OID4C
  • DID updates are going to did:web probably
  • Assertions will feature a different mechanism probably

OK

@gerardsn
Copy link
Member

gerardsn commented Jun 2, 2023

From the time.Truncate docs

	t, _ := time.Parse("2006 Jan 02 15:04:05", "2012 Dec 07 12:15:30.918273645")
        ...
	// To round to the last midnight in the local timezone, create a new Date.
	midnight := time.Date(t.Year(), t.Month(), t.Day(), 0, 0, 0, 0, time.Local)

not sure if it is worth the extra complexity, but this seems to be the way to truncate the 24h resolution the 30 day graph

Copy link
Member

@reinkrul reinkrul left a comment

Choose a reason for hiding this comment

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

Didn't review custom sliding window.

var transactions [3][]DataPoint

for i, window := range s.slidingWindows {
transactions[i] = window.dataPoints
Copy link
Member

Choose a reason for hiding this comment

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

still not protected?

data/window.go Outdated Show resolved Hide resolved
data/window.go Show resolved Hide resolved
data/window.go Outdated Show resolved Hide resolved
@woutslakhorst woutslakhorst requested a review from gerardsn June 7, 2023 10:26
@woutslakhorst woutslakhorst merged commit 3bc34a0 into main Jun 7, 2023
@woutslakhorst woutslakhorst deleted the feature/36/transactional_stats branch June 7, 2023 14:32
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