search

streamshub / flink-sql-examples

Apache License 2.0
1 stars 6 forks source link

add prometheus integration in recommendation-app #19

Closed showuon closed 2 weeks ago

showuon commented 2 weeks ago

This PR adds the prometheus integration in recommendation-app. This is for local environment demo use. Will add the integration with Openshift cluster built-in Prometheus examples in a separate PR.

showuon commented 2 weeks ago

@katheris @tinaselenge , please take a look when available. Thanks.

showuon commented 2 weeks ago

Thanks for the review @katheris !

I tried these and found a couple of updates were needed to work on Mac and also a few of the commands were missing -n flink.

Ah, right! I was testing in Ubuntu, and in my env, I set default namespace as flink, that's why I forgot to add -n flink. Sorry!

More generally I wonder whether the instructions for deploying Prometheus should be in the recommendation-app README, or whether it would be better putting them in the top level README? Do you think we could write the instructions generically so if we added other examples in future it would explain how to use prometheus with any of the examples?

Good point! I've moved the prometheus-install into root folder, and reference it in recommendation-app readme. Let me know if you have other suggestion. Thank you.

showuon commented 2 weeks ago

@katheris , thanks for the review. I've addressed your comments. Besides, I also added the steps to integrate with Openshift prometheus in this commit: https://github.com/streamshub/flink-sql-examples/pull/19/commits/eb267dddef72b6722da07cc9bb9f39e41ed19930 . Please have another look when available. Thanks.

showuon commented 2 weeks ago

@katheris @tinaselenge , thanks for the review. Addressed the comments. Thanks.

showuon commented 2 weeks ago

@kornys , do you have any other comment?

kornys commented 2 weeks ago

@showuon no, thank you for the fix and PR.