[exporter/loki] Add Config QueueSettings validation #16855

mar4uk · 2022-12-12T11:39:53Z

Description: Added QueueStettings validation into Config Validate method

Link to tracking Issue: #7841

Testing: Added unit tests

runforesight · 2022-12-12T11:41:06Z

Foresight Summary

Major Impacts

build-and-test-windows duration(4 seconds) has decreased 44 minutes 29 seconds compared to main branch avg(44 minutes 33 seconds).

View More Details

⭕ build-and-test-windows workflow has finished in 4 seconds (44 minutes 29 seconds less than `main` branch avg.) and finished at 9th Jan, 2023.

Job	Failed Steps	Tests
windows-unittest-matrix	- 🔗	N/A	See Details
windows-unittest	- 🔗	N/A	See Details

✅ tracegen workflow has finished in 4 minutes 10 seconds (⚠️ 1 minute 7 seconds more than `main` branch avg.) and finished at 9th Jan, 2023.

Job	Failed Steps	Tests
build-dev	- 🔗	N/A	See Details
publish-latest	- 🔗	N/A	See Details
publish-stable	- 🔗	N/A	See Details

✅ check-links workflow has finished in 4 minutes 40 seconds (⚠️ 2 minutes 1 second more than `main` branch avg.) and finished at 9th Jan, 2023.

Job	Failed Steps	Tests
changed files	- 🔗	N/A	See Details
check-links	- 🔗	N/A	See Details

✅ changelog workflow has finished in 5 minutes 21 seconds (4 minutes 7 seconds less than `main` branch avg.) and finished at 9th Jan, 2023.

Job	Failed Steps	Tests
changelog	- 🔗	N/A	See Details

✅ build-and-test workflow has finished in 44 minutes 46 seconds and finished at 9th Jan, 2023.

Job	Failed Steps	Tests
unittest-matrix (1.18, internal)	- 🔗	✅ 618 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, internal)	- 🔗	✅ 618 ❌ 0 ⏭ 0 🔗	See Details
correctness-metrics	- 🔗	✅ 2 ❌ 0 ⏭ 0 🔗	See Details
correctness-traces	- 🔗	✅ 17 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, processor)	- 🔗	✅ 1472 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, extension)	- 🔗	✅ 528 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, processor)	- 🔗	✅ 1472 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, extension)	- 🔗	✅ 528 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, receiver-0)	- 🔗	✅ 2565 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, receiver-0)	- 🔗	✅ 2565 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, exporter)	- 🔗	✅ 2466 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, other)	- 🔗	✅ 4435 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, exporter)	- 🔗	✅ 2466 ❌ 0 ⏭ 0 🔗	See Details
integration-tests	- 🔗	✅ 55 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, other)	- 🔗	✅ 4435 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.19, receiver-1)	- 🔗	✅ 1883 ❌ 0 ⏭ 0 🔗	See Details
unittest-matrix (1.18, receiver-1)	- 🔗	✅ 1883 ❌ 0 ⏭ 0 🔗	See Details
setup-environment	- 🔗	N/A	See Details
check-collector-module-version	- 🔗	N/A	See Details
check-codeowners	- 🔗	N/A	See Details
build-examples	- 🔗	N/A	See Details
checks	- 🔗	N/A	See Details
lint-matrix (receiver-0)	- 🔗	N/A	See Details
lint-matrix (receiver-1)	- 🔗	N/A	See Details
lint-matrix (processor)	- 🔗	N/A	See Details
lint-matrix (exporter)	- 🔗	N/A	See Details
lint-matrix (extension)	- 🔗	N/A	See Details
lint-matrix (internal)	- 🔗	N/A	See Details
lint-matrix (other)	- 🔗	N/A	See Details
unittest (1.19)	- 🔗	N/A	See Details
unittest (1.18)	- 🔗	N/A	See Details
lint	- 🔗	N/A	See Details
cross-compile (darwin, amd64)	- 🔗	N/A	See Details
cross-compile (darwin, arm64)	- 🔗	N/A	See Details
cross-compile (linux, 386)	- 🔗	N/A	See Details
cross-compile (linux, amd64)	- 🔗	N/A	See Details
cross-compile (linux, arm)	- 🔗	N/A	See Details
cross-compile (linux, ppc64le)	- 🔗	N/A	See Details
cross-compile (linux, arm64)	- 🔗	N/A	See Details
cross-compile (windows, amd64)	- 🔗	N/A	See Details
cross-compile (windows, 386)	- 🔗	N/A	See Details
build-package (deb)	- 🔗	N/A	See Details
build-package (rpm)	- 🔗	N/A	See Details
windows-msi	- 🔗	N/A	See Details
publish-check	- 🔗	N/A	See Details
publish-dev	- 🔗	N/A	See Details
publish-stable	- 🔗	N/A	See Details

✅ prometheus-compliance-tests workflow has finished in 15 minutes 7 seconds (⚠️ 6 minutes 29 seconds more than `main` branch avg.) and finished at 9th Jan, 2023.

Job	Failed Steps	Tests
prometheus-compliance-tests	- 🔗	✅ 21 ❌ 0 ⏭ 0 🔗	See Details

✅ load-tests workflow has finished in 21 minutes 47 seconds (⚠️ 6 minutes 3 seconds more than `main` branch avg.) and finished at 9th Jan, 2023.

Job	Failed Steps	Tests
loadtest (TestTraceAttributesProcessor)	- 🔗	✅ 3 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestIdleMode)	- 🔗	✅ 1 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestMetric10kDPS\|TestMetricsFromFile)	- 🔗	✅ 6 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestTraceNoBackend10kSPS\|TestTrace1kSPSWithAttrs)	- 🔗	✅ 8 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestMetricResourceProcessor\|TestTrace10kSPS)	- 🔗	✅ 12 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestTraceBallast1kSPSWithAttrs\|TestTraceBallast1kSPSAddAttrs)	- 🔗	✅ 10 ❌ 0 ⏭ 0 🔗	See Details
loadtest (TestBallastMemory\|TestLog10kDPS)	- 🔗	✅ 19 ❌ 0 ⏭ 0 🔗	See Details
setup-environment	- 🔗	N/A	See Details

🔎 See details on Foresight

^{*You can configure Foresight comments in your organization settings page.}

kovrus · 2022-12-12T12:24:40Z

We probably need to add Skip changelog label

evan-bradley

Looks good to me. It's worth mentioning that all of the sub keys are currently implicitly validated, as the Component interface will call Validate on any config structs that implement the method.

evan-bradley · 2022-12-16T17:22:10Z

exporter/lokiexporter/config.go

@@ -53,6 +53,10 @@ type Config struct {
 }

 func (c *Config) Validate() error {
+	if err := c.QueueSettings.Validate(); err != nil {
+		return fmt.Errorf("queue settings has invalid configuration: %w", err)


Nitpick, but I think calling out the failing config key here would be helpful.

the failing key could be QueueSize only, and it will be mentioned in the inner error message: https://github.com/open-telemetry/opentelemetry-collector/blob/7d168dd20efd4ebf659ccf7c501fdf02d7ed6dfd/exporter/exporterhelper/queued_retry.go#L76

So the error message here will be:
queue settings has invalid configuration: queue size must be positive

I think the message identifies the failing key clearly. Or maybe I misunderstood your concern?

codeboten

Please add a changelog as the user facing behaviour would be changed here.

github-actions · 2023-01-04T05:17:22Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

mar4uk · 2023-01-17T09:58:49Z

The changelog was added.
@codeboten could you please take a look once again?

mar4uk requested a review from a team December 12, 2022 11:39

mar4uk requested a review from jpkrohling as a code owner December 12, 2022 11:39

github-actions bot assigned evan-bradley Dec 12, 2022

github-actions bot added the exporter/loki Loki Exporter label Dec 12, 2022

github-actions bot requested review from gouthamve, gramidt and kovrus December 12, 2022 11:40

kovrus approved these changes Dec 12, 2022

View reviewed changes

mar4uk force-pushed the lokiexporter_validate_config branch from 9c71bb3 to 76026f7 Compare December 12, 2022 15:09

evan-bradley approved these changes Dec 16, 2022

View reviewed changes

evan-bradley added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Dec 16, 2022

codeboten reviewed Dec 20, 2022

View reviewed changes

codeboten removed the Skip Changelog PRs that do not require a CHANGELOG.md entry label Dec 20, 2022

github-actions bot added the Stale label Jan 4, 2023

[exporter/loki] Add Config QueueSettings validation

4346911

mar4uk force-pushed the lokiexporter_validate_config branch from 76026f7 to 420275f Compare January 9, 2023 20:20

added changelog

420275f

mar4uk requested review from codeboten and removed request for gramidt and gouthamve January 9, 2023 21:06

github-actions bot removed the Stale label Jan 10, 2023

codeboten approved these changes Jan 17, 2023

View reviewed changes

codeboten merged commit 3f56ff9 into open-telemetry:main Jan 17, 2023

codeboten mentioned this pull request Jan 17, 2023

[exporter/loki] Ensure Validate implementation does the right thing #7841

Closed

mar4uk mentioned this pull request Mar 22, 2023

REQUEST: New membership for mar4uk open-telemetry/community#1390

Closed

6 tasks

plantfansam mentioned this pull request Jul 21, 2023

disregard #24427

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[exporter/loki] Add Config QueueSettings validation #16855

[exporter/loki] Add Config QueueSettings validation #16855

mar4uk commented Dec 12, 2022

runforesight bot commented Dec 12, 2022 •

edited

Loading

`build-and-test-windows` duration(4 seconds) has decreased 44 minutes 29 seconds compared to main branch avg(44 minutes 33 seconds).

⭕ build-and-test-windows workflow has finished in 4 seconds (44 minutes 29 seconds less than `main` branch avg.) and finished at 9th Jan, 2023.

✅ tracegen workflow has finished in 4 minutes 10 seconds (⚠️ 1 minute 7 seconds more than `main` branch avg.) and finished at 9th Jan, 2023.

✅ check-links workflow has finished in 4 minutes 40 seconds (⚠️ 2 minutes 1 second more than `main` branch avg.) and finished at 9th Jan, 2023.

✅ changelog workflow has finished in 5 minutes 21 seconds (4 minutes 7 seconds less than `main` branch avg.) and finished at 9th Jan, 2023.

✅ build-and-test workflow has finished in 44 minutes 46 seconds and finished at 9th Jan, 2023.

✅ prometheus-compliance-tests workflow has finished in 15 minutes 7 seconds (⚠️ 6 minutes 29 seconds more than `main` branch avg.) and finished at 9th Jan, 2023.

✅ load-tests workflow has finished in 21 minutes 47 seconds (⚠️ 6 minutes 3 seconds more than `main` branch avg.) and finished at 9th Jan, 2023.

kovrus commented Dec 12, 2022 •

edited

Loading

evan-bradley left a comment

evan-bradley Dec 16, 2022

mar4uk Jan 9, 2023

codeboten left a comment

github-actions bot commented Jan 4, 2023

mar4uk commented Jan 17, 2023

[exporter/loki] Add Config QueueSettings validation #16855

[exporter/loki] Add Config QueueSettings validation #16855

Conversation

mar4uk commented Dec 12, 2022

runforesight bot commented Dec 12, 2022 • edited Loading

Foresight Summary

build-and-test-windows duration(4 seconds) has decreased 44 minutes 29 seconds compared to main branch avg(44 minutes 33 seconds).

⭕ build-and-test-windows workflow has finished in 4 seconds (44 minutes 29 seconds less than main branch avg.) and finished at 9th Jan, 2023.

✅ tracegen workflow has finished in 4 minutes 10 seconds (⚠️ 1 minute 7 seconds more than main branch avg.) and finished at 9th Jan, 2023.

✅ check-links workflow has finished in 4 minutes 40 seconds (⚠️ 2 minutes 1 second more than main branch avg.) and finished at 9th Jan, 2023.

✅ changelog workflow has finished in 5 minutes 21 seconds (4 minutes 7 seconds less than main branch avg.) and finished at 9th Jan, 2023.

✅ build-and-test workflow has finished in 44 minutes 46 seconds and finished at 9th Jan, 2023.

✅ prometheus-compliance-tests workflow has finished in 15 minutes 7 seconds (⚠️ 6 minutes 29 seconds more than main branch avg.) and finished at 9th Jan, 2023.

✅ load-tests workflow has finished in 21 minutes 47 seconds (⚠️ 6 minutes 3 seconds more than main branch avg.) and finished at 9th Jan, 2023.

kovrus commented Dec 12, 2022 • edited Loading

evan-bradley left a comment

Choose a reason for hiding this comment

evan-bradley Dec 16, 2022

Choose a reason for hiding this comment

mar4uk Jan 9, 2023

Choose a reason for hiding this comment

codeboten left a comment

Choose a reason for hiding this comment

github-actions bot commented Jan 4, 2023

mar4uk commented Jan 17, 2023

runforesight bot commented Dec 12, 2022 •

edited

Loading

`build-and-test-windows` duration(4 seconds) has decreased 44 minutes 29 seconds compared to main branch avg(44 minutes 33 seconds).

⭕ build-and-test-windows workflow has finished in 4 seconds (44 minutes 29 seconds less than `main` branch avg.) and finished at 9th Jan, 2023.

✅ tracegen workflow has finished in 4 minutes 10 seconds (⚠️ 1 minute 7 seconds more than `main` branch avg.) and finished at 9th Jan, 2023.

✅ check-links workflow has finished in 4 minutes 40 seconds (⚠️ 2 minutes 1 second more than `main` branch avg.) and finished at 9th Jan, 2023.

✅ changelog workflow has finished in 5 minutes 21 seconds (4 minutes 7 seconds less than `main` branch avg.) and finished at 9th Jan, 2023.

✅ prometheus-compliance-tests workflow has finished in 15 minutes 7 seconds (⚠️ 6 minutes 29 seconds more than `main` branch avg.) and finished at 9th Jan, 2023.

✅ load-tests workflow has finished in 21 minutes 47 seconds (⚠️ 6 minutes 3 seconds more than `main` branch avg.) and finished at 9th Jan, 2023.

kovrus commented Dec 12, 2022 •

edited

Loading