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

Fix k8s pulsar functions containers not exposing metrics port for scraping #12065

Merged

Conversation

volgorean
Copy link
Contributor

Motivation

  • We are currently unable to scrape metrics from pulsar functions pods on k8s because the metrics port is not exposed.

Modifications

  • updates getFunctionContainerPorts to return both its grpc port as well as the metric port (currently the function returning the metrics port in its own list is not being called)

Verifying this change

  • Make sure that the change passes the CI checks.

This change added tests and can be verified as follows:

  • Deploy pulsar to k8s (or update the image of an existing deployment) w/ functions enabled
  • inspect the functions pods to see that they now expose a metrics port along with their current grpc port

Documentation

  • no-need-doc (minor fix for something that should already be enabled)

@volgorean
Copy link
Contributor Author

@sijie would you mind approving the workflows/tests to be run? 🙏

@Anonymitaet Anonymitaet added the doc-not-needed Your PR changes do not impact docs label Sep 18, 2021
@smazurov
Copy link
Contributor

can this be reviewed/merged?

Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Copy link
Member

@michaeljmarshall michaeljmarshall left a comment

Choose a reason for hiding this comment

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

LGTM

@michaeljmarshall
Copy link
Member

Closing and reopening to trigger CI. All of the tests are in "Expected" state. I haven't seen this happen before.

@michaeljmarshall
Copy link
Member

Alright, tests are listed as "Queued", so they should run.

@michaeljmarshall
Copy link
Member

/pulsarbot run-failure-checks

@michaeljmarshall
Copy link
Member

michaeljmarshall commented Dec 3, 2021

Unfortunately, we have some flaky tests. The test that failed is not related to this change. Re-running the failed test.

@michaeljmarshall michaeljmarshall added this to the 2.10.0 milestone Dec 3, 2021
@michaeljmarshall michaeljmarshall merged commit 4dd4bd6 into apache:master Dec 3, 2021
michaeljmarshall pushed a commit that referenced this pull request Dec 3, 2021
@michaeljmarshall michaeljmarshall added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Dec 3, 2021
michaeljmarshall pushed a commit that referenced this pull request Dec 3, 2021
@michaeljmarshall
Copy link
Member

@volgorean - thanks for your contribution! I cherry picked it to the relevant branches.

nicoloboschi pushed a commit to datastax/pulsar that referenced this pull request Dec 3, 2021
fxbing pushed a commit to fxbing/pulsar that referenced this pull request Dec 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/function cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.2 release/2.9.1 type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants