[#9177] add metrics server to go function #9318
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
zymap
reviewed
Jan 26, 2021
| @@ -119,7 +119,8 @@ | |||
| public static List<String> getGoInstanceCmd(InstanceConfig instanceConfig, | |||
| String originalCodeFileName, | |||
| String pulsarServiceUrl, | |||
| boolean k8sRuntime) throws IOException { | |||
| boolean k8sRuntime, | |||
| int metricsPort) throws IOException { | |||
There was a problem hiding this comment.
Why not introduce the new configuration in the InstanceConfig?
There was a problem hiding this comment.
I havnt add metricsPort to InstanceConfig is because metricsPort is generated with different mechanisms for different runtime, such as for process runtime, metricsPort is generated by FunctionCommon.findAvailablePort() but for k8s runtime, metricsPort comes from the config functions_worker.yml.
Also, for java and python function, metricsPort is also not included in InstanceConfig (ref:
I did seem a bit lazy with these changes here, but also want to avoid making changes to java and python runtime's code. So let me know if we want apply metricsPort into InstanceConfig.
freeznet
force-pushed
the
freeznet/fix-golang-function-metrics-port-error
branch
from
cd65b4c
to
2676bd5
Compare
|
/pulsarbot run-failure-checks |
1 similar comment
|
/pulsarbot run-failure-checks |
freeznet
force-pushed
the
freeznet/fix-golang-function-metrics-port-error
branch
from
2676bd5
to
f1a7572
Compare
|
/pulsarbot run-failure-checks |
1 similar comment
|
/pulsarbot run-failure-checks |
freeznet
force-pushed
the
freeznet/fix-golang-function-metrics-port-error
branch
from
f1a7572
to
33b7258
Compare
|
@wolfstudy thanks for review. I will go for @zymap and your advice. But this will affect java and python's code, should I put |
|
Yes, make sense to me. Let's get merge this PR. |
|
/pulsarbot run-failure-checks |
zymap
approved these changes
Jan 29, 2021
|
/pulsarbot run-failure-checks |
sijie
approved these changes
Jan 29, 2021
jiazhai
approved these changes
Jan 29, 2021
freeznet
force-pushed
the
freeznet/fix-golang-function-metrics-port-error
branch
from
33b7258
to
a7f52b5
Compare
|
/pulsarbot run-failure-checks |
codelipenghui
approved these changes
Jan 30, 2021
1 task
sijie
pushed a commit
that referenced
this pull request
Feb 2, 2021
### Motivation #9318 add metrics server to go function, but didnt serve `"/"` endpoint in metrics server, which will cause some metrics calls failed. ### Modifications add handler for `"/"` in `MetricsServicer` ### Verifying this change - [x] Make sure that the change passes the CI checks.
Anonymitaet
pushed a commit
that referenced
this pull request
Feb 3, 2021
### Motivation #9318 add metrics server to go function, but didnt serve `"/"` endpoint in metrics server, which will cause some metrics calls failed. ### Modifications add handler for `"/"` in `MetricsServicer` ### Verifying this change - [x] Make sure that the change passes the CI checks.
codelipenghui
pushed a commit
that referenced
this pull request
Feb 4, 2021
Fixes #9177 go function added metrics collector by #6105, but havnt pass `metricsPort` to go function, also not init & start prometheus http server. As the result, function worker will keep trying to access to the metrics port to collect data, which will cause massive log errors in log history. - expose `metricsPort` to go function - add prometheus http server to go function - [x] Make sure that the change passes the CI checks. (cherry picked from commit 211a125)
codelipenghui
pushed a commit
that referenced
this pull request
Feb 4, 2021
### Motivation #9318 add metrics server to go function, but didnt serve `"/"` endpoint in metrics server, which will cause some metrics calls failed. ### Modifications add handler for `"/"` in `MetricsServicer` ### Verifying this change - [x] Make sure that the change passes the CI checks. (cherry picked from commit c99e1a0)
1 task
zymap
pushed a commit
that referenced
this pull request
Feb 22, 2021
Master Issue: #9177 ### Motivation As discussed in #9318, both @zymap and @wolfstudy suggested, to add `metricsPort` as a field of `InstanceConfig`. ### Modifications - add metricsPort to InstanceConfig - add hasValidMetricsPort to InstanceConfig to check if metrics port is valid - applied changes to k8s runtime & process runtime
|
@freeznet Thank you for fixing this issue! |
ivankelly
pushed a commit
to ivankelly/pulsar
that referenced
this pull request
Aug 10, 2021
Master Issue: apache#9177 As discussed in apache#9318, both @zymap and @wolfstudy suggested, to add `metricsPort` as a field of `InstanceConfig`. - add metricsPort to InstanceConfig - add hasValidMetricsPort to InstanceConfig to check if metrics port is valid - applied changes to k8s runtime & process runtime
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #9177
Motivation
go function added metrics collector by #6105, but havnt pass
metricsPortto go function, also not init & start prometheus http server. As the result, function worker will keep trying to access to the metrics port to collect data, which will cause massive log errors in log history.Modifications
metricsPortto go functionVerifying this change