Skip to content

Export string sets in monitoring infos.#31838

Merged
robertwb merged 3 commits intoapache:masterfrom
robertwb:string-set-v2
Jul 11, 2024
Merged

Export string sets in monitoring infos.#31838
robertwb merged 3 commits intoapache:masterfrom
robertwb:string-set-v2

Conversation

@robertwb
Copy link
Copy Markdown
Contributor

This ensures they'll be available on Dataflow Runner v2.


Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

  • Mention the appropriate issue in your description (for example: addresses #123), if applicable. This will automatically add a link to the pull request in the issue. If you would like the issue to automatically close on merging the pull request, comment fixes #<ISSUE NUMBER> instead.
  • Update CHANGES.md with noteworthy changes.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

See the Contributor Guide for more tips on how to make review process smoother.

To check the build health, please visit https://github.com/apache/beam/blob/master/.test-infra/BUILD_STATUS.md

GitHub Actions Tests Status (on master branch)

Build python source distribution and wheels
Python tests
Java tests
Go tests

See CI.md for more information about GitHub Actions CI or the workflows README to see a list of phrases to trigger workflows.

@robertwb
Copy link
Copy Markdown
Contributor Author

R: @Abacn

@github-actions
Copy link
Copy Markdown
Contributor

Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment assign set of reviewers

@Abacn
Copy link
Copy Markdown
Contributor

Abacn commented Jul 11, 2024

Checkout this PR, removed excludeCategory in commonRunnerV2ExcludeCategories and ran ./gradlew :runners:google-cloud-dataflow-java:validatesRunnerV2Test --tests *test*StringSetMetrics manually, still get the same AssertionError

org.apache.beam.sdk.metrics.MetricsTest$AttemptedMetricTests

java.lang.AssertionError: 
Expected: iterable with items [MetricResult{inNamespace="org.apache.beam.sdk.metrics.MetricsTest", name="sources", step="MyStep1", attempted=<StringSetResult{stringSet=[gcs]}>}, MetricResult{inNamespace="org.apache.beam.sdk.metrics.MetricsTest", name="sinks", step="MyStep2", attempted=<StringSetResult{stringSet=[kafka, bq]}>}, MetricResult{inNamespace="org.apache.beam.sdk.metrics.MetricsTest", name="sideinputs", step="MyStep1", attempted=<StringSetResult{stringSet=[bigtable, spanner]}>}, MetricResult{inNamespace="org.apache.beam.sdk.metrics.MetricsTest", name="sideinputs", step="MyStep2", attempted=<StringSetResult{stringSet=[sql, bigtable]}>}] in any order
     but: no item matches: MetricResult{inNamespace="org.apache.beam.sdk.metrics.MetricsTest", name="sources", step="MyStep1", attempted=<StringSetResult{stringSet=[gcs]}>}, MetricResult{inNamespace="org.apache.beam.sdk.metrics.MetricsTest", name="sinks", step="MyStep2", attempted=<StringSetResult{stringSet=[kafka, bq]}>}, MetricResult{inNamespace="org.apache.beam.sdk.metrics.MetricsTest", name="sideinputs", step="MyStep1", attempted=<StringSetResult{stringSet=[bigtable, spanner]}>}, MetricResult{inNamespace="org.apache.beam.sdk.metrics.MetricsTest", name="sideinputs", step="MyStep2", attempted=<StringSetResult{stringSet=[sql, bigtable]}>} in []
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:18)
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
	at org.apache.beam.sdk.metrics.MetricsTest.assertStringSetMetrics(MetricsTest.java:449)

is this expected? (there is still some gap to make this metrics available in PipelineResult

@robertwb
Copy link
Copy Markdown
Contributor Author

is this expected? (there is still some gap to make this metrics available in PipelineResult

Yes, there's still work to plumb the results back on the Dataflow service end.

Copy link
Copy Markdown
Contributor

@Abacn Abacn left a comment

Choose a reason for hiding this comment

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

thanks

}
});
stringSets.forEach(
(metricName, gaugeCell) -> {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

minor: gaugeCell -> stringSetSell

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done.

@robertwb robertwb merged commit 50a3403 into apache:master Jul 11, 2024
robertwb added a commit to robertwb/incubator-beam that referenced this pull request Jul 11, 2024
acrites pushed a commit to acrites/beam that referenced this pull request Jul 17, 2024
reeba212 pushed a commit to reeba212/beam that referenced this pull request Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants