expose prometheus metrics + cloudrun compatible #24

roshaans · 2022-11-17T21:30:55Z

This PR enables us to look into the performance of the micro-indexers and makes them compatible with GCP's Cloud Run service.

Works in Cloudrun
Prometheus metrics /metrics
- Block Processing Speed
- Latest Block Height indexed by Indexer
Nonblocking JSON logs

src/configs.rs

src/main.rs

src/metrics_server.rs

telezhnaya

Please address the comments and I'll make the review
My idealism and tediousness don't allow me to concentrate on the main things

src/configs.rs

src/main.rs

Cargo.lock

telezhnaya

I haven't seen this in action. After you address the comments, please help @ecp88 to launch this

Cargo.toml

src/main.rs

src/metrics_server.rs

src/tracing_utils.rs

src/metrics_server.rs

telezhnaya

Could you please re-check the messages that we already have in the PR?
E.g., this one is ignored
https://github.com/near/near-indexer-events/pull/24/files#r1031328820
After you fix the issue, please mark the message as resolved. If you doubt in the proposed solution, leave the thread unresolved and ask any questions/leave the comments in the same thread

Also, please have a look at khorolets/near-indexer-for-explorer#14 , it's better to use the same approach

roshaans · 2022-11-30T10:46:19Z

Could you please re-check the messages that we already have in the PR? E.g., this one is ignored https://github.com/near/near-indexer-events/pull/24/files#r1031328820 After you fix the issue, please mark the message as resolved. If you doubt in the proposed solution, leave the thread unresolved and ask any questions/leave the comments in the same thread

Also, please have a look at khorolets/near-indexer-for-explorer#14 , it's better to use the same approach

Thank you @telezhnaya! I'll keep this in mind for future PRs. Unfortunately, I do not think we can use the same approach as in this PR. The Default provider searches in .aws/credentials for credential details, while we need to be able to pass in these details as environment variables so we can configure it from Cloud Run for example.

I pushed another update to this PR which hides setting up configuration details from the main.rs file. You can view that change here -> https://github.com/near/near-indexer-events/pull/24/files#r1035819281

Update: Yes we can use this approach. We just need to make sure the env variables comply with what the AWS credentials provider expects.
"The default credentials provider will also source from the environment, but it looks for AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY, the sourcing process is described here https://docs.aws.amazon.com/sdk-for-rust/latest/dg/credentials.html." - Morgan

telezhnaya · 2022-11-30T10:52:24Z

In this case, we have to revert khorolets/near-indexer-for-explorer#14 as well, right? We will have the same deploy process anyway
cc @morgsmccauley

src/main.rs

roshaans · 2022-11-30T10:56:10Z

In this case, we have to revert khorolets/near-indexer-for-explorer#14 as well, right? We will have the same deploy process anyway cc @morgsmccauley

Yes, that is correct

src/configs.rs

morgsmccauley · 2022-11-30T16:51:53Z

The Default provider searches in .aws/credentials for credential details, while we need to be able to pass in these details as environment variables

@telezhnaya @roshaans The default credentials provider will also source from the environment, but note that it looks for AWS_ACCESS_KEY_ID and AWS_SECRET_ACCESS_KEY as described here

telezhnaya

Thank you for smaller set of changes!

I kindly ask you further not to ignore the comments I'm leaving. I'm waiting that you either

fix the issue (and try to remember the idea and not to repeat such issues further)
OR
argue and explain your approach (I highly welcome this)

As any other reviewer, I feel demotivated when I have to repeat the same comments, especially in the same review at the same lines.

Dockerfile

Cargo.toml

src/configs.rs

telezhnaya · 2022-12-02T11:45:57Z

src/main.rs

 }

 async fn handle_streamer_message(
    streamer_message: near_indexer_primitives::StreamerMessage,
    pool: &sqlx::Pool<sqlx::Postgres>,
 ) -> anyhow::Result<u64> {
+    metrics::BLOCK_PROCESSED_TOTAL.inc();
+    metrics::LATEST_BLOCK_HEIGHT.set(streamer_message.block.header.height.try_into().unwrap());


I've already told you about unwrap in general and this cast in particular.
It gives unreadable error, we won't be able to investigate this in production.
Moreover, here you silently do the cast that we actually can't do because u64 could not be safely casted to i64.
I asked you to leave the comment why we have to do this, and I'm asking you again. Please be more careful to the comments in the review.

I should have waited for your feedback before resolving the conversation -> #24 (comment)

I've added a comment and converted it in a way that it will propagate a meaningful error.
@morgsmccauley you might want to take care of this later
https://github.com/khorolets/near-indexer-for-explorer/blob/0bdd70e919c3781df9bf2dc900b2e2d73b5257cb/indexer/src/main.rs#L26

#24 (comment)

src/metrics.rs

Cargo.toml

telezhnaya · 2022-12-05T10:01:54Z

src/metrics.rs

+lazy_static! {
+    pub static ref BLOCK_PROCESSED_TOTAL: IntCounter = try_create_int_counter(
+        "indexer_events_total_blocks_processed",
+        "Total number of blocks processed by indexer regardless of restarts. Used to calculate Block Processing Rate(BPS)"


Much better, thank you!

src/main.rs

roshaans added 9 commits November 17, 2022 12:48

metrics server and collection

96adbe9

/metrics enabled

9d58f22

port changed from .env

e7d9c78

block processing metrics

83f88ca

changed port for cloud run compatability

cdc57a9

fix dependencies

d24b7ed

start options sourced from .env bucket

f73f96a

made credentials as arguments to program

364549c

added /probe to see if indexer is active

9526fbc