Skip to content
This repository was archived by the owner on Jan 28, 2022. It is now read-only.

Conversation

@storey247
Copy link
Contributor

  • Extended workflow of controllers to report upstream call times and counts
  • Identified a couple of issues with linting and patched as found
  • Refactored some code to make it more readable

@storey247 storey247 marked this pull request as ready for review November 13, 2019 09:53
Copy link
Contributor

@Azadehkhojandi Azadehkhojandi left a comment

Choose a reason for hiding this comment

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

Awesome work and I learnt a lot by reviewing your pr.
Could you please cherry-pick only changes related to metrics and apply the minor changes I requested?

@Azadehkhojandi
Copy link
Contributor

/azp list

@azure-pipelines
Copy link

CI/CD Pipelines for this repository:

@Azadehkhojandi
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@storey247
Copy link
Contributor Author

@Azadehkhojandi thanks for the great feedback, I've tidied things up now and actioned the feedback you requested.

The PR should now only contain the commits relevant to the metrics work I did (apologies for the confusion).

Looking forward to your feedback

@stuartleeks
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@storey247
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@storey247
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@stuartleeks stuartleeks force-pushed the f-metrics branch 3 times, most recently from e152cb6 to 8960a26 Compare December 13, 2019 08:47
@stuartleeks
Copy link
Collaborator

After our discussion earlier this week I have rebased this PR on latest master and dropped out most of the kustomize changes (which are essentially incorporated into the kubebuilder upgrade now in master) with a couple of minor tweaks left in to enable prometheus to connect.

I've tested this locally by doing the following in the devcontainer

  • uncomment - ../prometheus include in config/default/kustomize.yaml to enable prometheus (I've left it as not enabled by default)
  • make set-kindcluster to spin everything up in kind
  • kubectl apply -f config/samples/databricks_v1alpha1_run_direct.yaml to create a run
  • kubectl port-forward svc/prom-azure-databricks-oper-prometheus 9090:9090 to port-forward to prometheus service so that we can connect
  • Open http://localhost:9090 in my browser to get the prometheus UI and add a chart for databricks_run_submit_request_duration_seconds_count as shown below

image

@Azadehkhojandi Azadehkhojandi merged commit 49d44fa into Azure:master Dec 17, 2019
amrasem added a commit to amrasem/azure-databricks-operator that referenced this pull request Jan 15, 2020
Extended databricks operator to report metrics into Prometheus (Azure#104)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants