[receiver/Flinkmetricsreceiver] Apache Flink Metric Receiver #10121
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
JonathanWamsley
force-pushed
the
flinkmetricsreceiver
branch
2 times, most recently
from
d086817 to
5744c6e
Compare
antonblock
reviewed
May 18, 2022
JonathanWamsley
force-pushed
the
flinkmetricsreceiver
branch
2 times, most recently
from
32ced72 to
f8eed3a
Compare
djaglowski
reviewed
May 19, 2022
receiver/flinkmetricsreceiver/testdata/apiresponses/jobmanager_metric_names.json
Outdated
Show resolved
Hide resolved
receiver/flinkmetricsreceiver/testdata/apiresponses/jobmanager_metric_values.json
Outdated
Show resolved
Hide resolved
receiver/flinkmetricsreceiver/testdata/apiresponses/jobs_ids.json
Outdated
Show resolved
Hide resolved
receiver/flinkmetricsreceiver/testdata/apiresponses/jobs_metric_names.json
Outdated
Show resolved
Hide resolved
JonathanWamsley
force-pushed
the
flinkmetricsreceiver
branch
5 times, most recently
from
553ec66 to
1743f8f
Compare
djaglowski
reviewed
May 23, 2022
|
I updated pr feedback @djaglowski and it is ready to be looked at again please. |
djaglowski
reviewed
May 24, 2022
JonathanWamsley
force-pushed
the
flinkmetricsreceiver
branch
2 times, most recently
from
7f104f5 to
7465bfc
Compare
|
PR is ready for review again please @djaglowski |
djaglowski
reviewed
May 25, 2022
JonathanWamsley
force-pushed
the
flinkmetricsreceiver
branch
3 times, most recently
from
9e336d0 to
07cce9f
Compare
|
Ready to be looked at again @djaglowski please |
djaglowski
reviewed
May 26, 2022
| flink.jvm.cpu.load: | ||
| enabled: true | ||
| description: The CPU usage (%) of the JVM for a jobmanager or taskmanager. | ||
| unit: 1 |
There was a problem hiding this comment.
I'm confused. I thought you said this metric is measured in %. That's my interpretation from the article you linked.
There was a problem hiding this comment.
yeah, I could not find a unit though and searching through the metadata.yaml mongo atlas had that convention. so I put it in. I thought you were saying with "%?" to remove that convention or something.
There was a problem hiding this comment.
Ok, should the units be % or not?
There was a problem hiding this comment.
Yes they should be. The unit can be 1 but not % because that will break in the metadata generation. Maybe "{percentage}" instead?
There was a problem hiding this comment.
updated unit
djaglowski
reviewed
May 26, 2022
receiver/flinkmetricsreceiver/testdata/expected_metrics/metrics_golden.json
Show resolved
Hide resolved
JonathanWamsley
force-pushed
the
flinkmetricsreceiver
branch
from
07cce9f to
6eeaae2
Compare
JonathanWamsley
force-pushed
the
flinkmetricsreceiver
branch
from
83f006e to
7e49410
Compare
djaglowski
approved these changes
Jun 1, 2022
| enum: [ jobmanager, taskmanager ] | ||
|
|
||
| attributes: | ||
| operator_name: |
There was a problem hiding this comment.
This can be a followup issue, but I think this should probably be user-visible as name.
Suggested change
| operator_name: | |
| operator_name: | |
| value: name |
| operator_name: | ||
| description: The operator name. | ||
| type: string | ||
| garbage_collector_name: |
There was a problem hiding this comment.
Suggested change
| garbage_collector_name: | |
| garbage_collector_name: | |
| value: name |
kentquirk
pushed a commit
to McSick/opentelemetry-collector-contrib
that referenced
this pull request
Jun 14, 2022
…lemetry#10121) * wip before integration and concurrency * tidy up client names * wip integration test * wip integration test * update to flinkmetrics and integration test * move attributes to resource attributes * add flink integration files * update model import name * add changelog and tidy up * update mod and lint check * update with pr feedback * update pr feedback with status header * unexport process methods * go tidy up * add partial metrics collecting on taskmanager, jobs, and subtasks * go tidy update * fix shadow declaration of err * update pr feedback * tidy up * apply scope version update * make expected responses easier to test when metadata/generated files update * update to host.name and move status table * combine jobmanager and taskmanager metrics * tidy up * update generated files * fix mod version * update pr feedback * tidy up * seperated checkpoint in_progress as non-monotonic sum metric * tidy up local receiver * update version * update cpu descriptions * fix taskmanager/jobs bug and added tests with multiple taskmanagers/jobs * space * change non_heap to nonheap * update unit
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.
Description: Add New Component: Apache Flink Metric Receiver
Link to tracking Issue: New Component: Apache Flink Metric Receiver
Testing: A series of testing was added including:
Documentation: README.md documentation.md