Skip to content

Metrics from Health handlers for Bots V2#844

Merged
canercidam merged 4 commits intomasterfrom
kisel/forta-1404-receive-metrics
Jan 29, 2024
Merged

Metrics from Health handlers for Bots V2#844
canercidam merged 4 commits intomasterfrom
kisel/forta-1404-receive-metrics

Conversation

@dkeysil
Copy link
Contributor

@dkeysil dkeysil commented Jan 18, 2024

Since bots v2 are sending alerts by themselves but no metrics - we need to introduce new way of sending metrics - by polling health handler to gather additional information.

These metrics then are going to be aggregated and indexed.

Changed:

  • Each metric now can contain a chain id (and one V2 bot can expose metrics related to many chains in the same time)
  • It's possible to pass metrics in addition to errors in the health handler

@dkeysil dkeysil force-pushed the kisel/forta-1404-receive-metrics branch 4 times, most recently from 62bb77e to d566444 Compare January 19, 2024 16:40
@dkeysil dkeysil marked this pull request as ready for review January 19, 2024 16:41
@dkeysil dkeysil requested review from aomerk and canercidam and removed request for canercidam January 19, 2024 16:41
@aomerk
Copy link
Contributor

aomerk commented Jan 22, 2024

i think it would be useful if we had an e2e test for this one:

  • choose one of the bots under tests/e2e/agents
  • implement health check
  • add a test case for health checks under e2e_forta_local_mode_test.go

@dkeysil dkeysil requested a review from canercidam January 29, 2024 07:07
@aomerk
Copy link
Contributor

aomerk commented Jan 29, 2024

Can we use metric name definitions from the core-go PR?

…AgentMetrics, supported metrics in the health request

Datapoints int64 -> float64

Bump forta-core-go to use int64 for chain id, add chain id to metrics that doesn't have it

Add metrics to bothttp/client_test

Add check for exceeded response size

Remove comment

Add chain id from bot config in lifecycle metrics

Remove unnecessary metric

Pass chain id from alert config, remove todo, fix disco configuration

Add TODO

Move chain id fallback to metrics aggregator

Use forta-core-go metrics names
@dkeysil dkeysil force-pushed the kisel/forta-1404-receive-metrics branch from ce146a4 to 13b0bd7 Compare January 29, 2024 17:54
@dkeysil dkeysil force-pushed the kisel/forta-1404-receive-metrics branch from 838e2a6 to 07de81d Compare January 29, 2024 18:34
s.botClient.StartProcessing()
s.botClient.Initialize()

<-healthCheckChan
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

bucket.MetricDetails[m.Name] = m.Details

// add chain id if it's not set, e.g. EventMetric
chainID := ama.chainID
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for moving this here.

@canercidam canercidam merged commit b067fd8 into master Jan 29, 2024
@canercidam canercidam deleted the kisel/forta-1404-receive-metrics branch January 29, 2024 18:55
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