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

Add Monitoring to test local performance improvements #211

Merged
merged 19 commits into from
Nov 6, 2024

Conversation

Jeffrey-Vervoort-KNMI
Copy link
Contributor

When running just local you can access the monitoring.

  • E.g. in Grafana you can view statistics about the gRPC methods
  • E.g. in Grafana you can view statistics about the most expensive and most called database queries.

Copy link

github-actions bot commented Oct 23, 2024

API Unit Test Coverage Report
FileStmtsMissCoverMissing
\_\_init\_\_.py00100% 
datastore_pb2.py614821%34–81
datastore_pb2_grpc.py542750%15–16, 19, 65–80, 121–123, 128–130, 135–137, 142–144, 148–173, 219, 246, 273, 300
export_metrics.py100100% 
grpc_getter.py201145%15–19, 23–26, 30–32, 36–38
locustfile.py15150%1–31
main.py43784%45, 50, 60, 70–71, 81–82
metadata_endpoints.py653152%45–54, 58, 85, 100–219, 223
response_classes.py50100% 
utilities.py1744674%20, 38, 45, 67–70, 78–89, 94–101, 121, 125, 127, 155, 161, 179, 193–194, 198, 214–218, 222–228, 232–234, 264, 268, 290, 295
custom_geo_json
   edr_feature_collection.py60100% 
formatters
   \_\_init\_\_.py110100% 
   covjson.py59198%91
   geojson.py21290%27, 52
openapi
   custom_dimension_examples.py40100% 
   edr_query_parameter_descriptions.py110100% 
   openapi_examples.py130100% 
routers
   \_\_init\_\_.py00100% 
   edr.py101496%348–349, 438–439
   feature.py471960%99–132, 148–153, 159–181
TOTAL72021171% 

API Unit Test Coverage Summary

Tests Skipped Failures Errors Time
30 0 💤 0 ❌ 0 🔥 1.824s ⏱️

@jo-asplin-met-no
Copy link
Contributor

jo-asplin-met-no commented Oct 28, 2024

The gRPC Server dashboard in Grafana has the same three panel groups (or whatever you call them) for each of three jobs. The panel groups are gRPC General, gRPC Methods, and Go, while the jobs are prometheus, prometheus-postgres-exporter, and store. Does this mean that prometheus and prometheus-postgres-exporter are also written in Go and use gRPC for communication?

@Jeffrey-Vervoort-KNMI
Copy link
Contributor Author

The gRPC Server dashboard in Grafana has the same three panel groups (or whatever you call them) for each of three jobs. The panel groups are gRPC General, gRPC Methods, and Go, while the jobs are prometheus, prometheus-postgres-exporter, and store.

I think the Grafana term is variables. At this moment, except for the interval, you always want to keep them at their default value. The other values are not that interesting. For example, database names is a dynamic list retrieved from Prometheus. It consists of all the databases in the Postgres container, namely our database and the default databases. I did not mind it, but for clarity, it could be better to only have data as value. What do you think?

Does this mean that prometheus and prometheus-postgres-exporter are also written in Go and use gRPC for communication?

They are written in Go. I don't think they use gRPC. I made the dashboard this way as I thought for our most common use case store everything is in one place. If we want to view the Go performance of "prometheus" or the "exporter" we can. But as I said I expect we seldom look at it, so I think there is something to say to change the variable to only store. What do you think?

@jo-asplin-met-no
Copy link
Contributor

jo-asplin-met-no commented Oct 28, 2024

To avoid confusion I would personally have preferred to move the two monitoring servers (prometheus and exporter) out in a separate dashboard (typically called Monitoring Infrastructure or something like that) and no longer associate them with gRPC metrics (since that's n/a as you say). Or even delete them altogheter, but that's not important I think. It's interesting to have them there.

…he extra.conf the extension is loaded on a new clean database (just destroy local). But it is not created for existing databases. Add a migration to create the extension.
Copy link
Contributor

@jo-asplin-met-no jo-asplin-met-no left a comment

Choose a reason for hiding this comment

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

Looks good!

@Jeffrey-Vervoort-KNMI Jeffrey-Vervoort-KNMI merged commit 63e2a3e into main Nov 6, 2024
8 checks passed
@Jeffrey-Vervoort-KNMI Jeffrey-Vervoort-KNMI deleted the logging branch November 6, 2024 15:02
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