Skip to content

Conversation

@umustafi
Copy link
Contributor

@umustafi umustafi commented Aug 4, 2021

*Emitted in the form <flowgroup>.<flowname>.workunits

Dear Gobblin maintainers,

Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!

JIRA

Description

  • Here are some details about my PR, including screenshots (if applicable):
    From a user/platform perspective, we want to see how many workunits are being created for each flow. The metric of workunit count will be emitted in the form ..workunits.

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:
    Tested with flows launched on gaas to see log statement "Reporting metrics for host: {}, app: {} to Ingraphs with reporting interval of {} seconds."

Commits

  • My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue. In addition, my commits follow the guidelines from "How to write a good git commit message":
    1. Subject is separated from body by a blank line
    2. Subject is limited to 50 characters
    3. Subject does not end with a period
    4. Subject uses the imperative mood ("add", not "adding")
    5. Body wraps at 72 characters
    6. Body explains "what" and "why", not "how"

@codecov-commenter
Copy link

codecov-commenter commented Aug 4, 2021

Codecov Report

Merging #3351 (a14fdb2) into master (de94515) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #3351      +/-   ##
============================================
- Coverage     46.43%   46.41%   -0.02%     
- Complexity    10120    10155      +35     
============================================
  Files          2052     2056       +4     
  Lines         79656    80102     +446     
  Branches       8896     8950      +54     
============================================
+ Hits          36991    37183     +192     
- Misses        39226    39472     +246     
- Partials       3439     3447       +8     
Impacted Files Coverage Δ
...rg/apache/gobblin/runtime/AbstractJobLauncher.java 61.11% <ø> (-0.15%) ⬇️
...blin/service/monitoring/KafkaJobStatusMonitor.java 35.57% <0.00%> (-7.45%) ⬇️
...leshooter/InMemoryMultiContextIssueRepository.java 60.00% <0.00%> (-35.24%) ⬇️
...a/org/apache/gobblin/util/limiter/NoopLimiter.java 40.00% <0.00%> (-20.00%) ⬇️
...g/apache/gobblin/util/filters/RegexPathFilter.java 72.72% <0.00%> (-16.17%) ⬇️
...lin/util/filesystem/FileSystemInstrumentation.java 85.71% <0.00%> (-14.29%) ⬇️
...e/gobblin/runtime/local/LocalTaskStateTracker.java 72.97% <0.00%> (-7.59%) ⬇️
...t/version/TimestampedDatasetStateStoreVersion.java 27.77% <0.00%> (-5.56%) ⬇️
...in/java/org/apache/gobblin/cluster/HelixUtils.java 32.23% <0.00%> (-2.48%) ⬇️
...ore/util/DatabaseJobHistoryStoreSchemaManager.java 29.31% <0.00%> (-2.36%) ⬇️
... and 55 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update de94515...a14fdb2. Read the comment docs.

@umustafi
Copy link
Contributor Author

umustafi commented Aug 4, 2021

@arjun4084346 @jack-moseley can you review?

@umustafi umustafi force-pushed the emitWorkUnitCountMetrics branch from f8aaa7b to d9fe511 Compare August 5, 2021 19:25
@umustafi umustafi force-pushed the emitWorkUnitCountMetrics branch 2 times, most recently from 16e5839 to 63ede5f Compare August 9, 2021 22:59
	*Emitted in the form <flowgroup>.<flowname>.WorkUnitsCreated
@umustafi umustafi force-pushed the emitWorkUnitCountMetrics branch from 63ede5f to 363c8e7 Compare August 10, 2021 00:44
@arjun4084346
Copy link
Contributor

+1 LGTM but not sure about the necessity of this metrics.

@umustafi
Copy link
Contributor Author

@sv2000 can you merge?

@umustafi
Copy link
Contributor Author

@aplex can you merge?

@aplex aplex merged commit 2b36f27 into apache:master Aug 26, 2021
@umustafi umustafi deleted the emitWorkUnitCountMetrics branch August 26, 2021 00:33
jack-moseley pushed a commit to jack-moseley/gobblin that referenced this pull request Aug 24, 2022
From a user/platform perspective, we want to see how many workunits are being created for each flow. The metric of workunit count will be emitted in the form [flow-group].[flow-name].workunits.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants