-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[receiver/splunkhec] Drop events when nested indexed fields are present #18068
Conversation
Foresight Summary
View More Details✅ tracegen workflow has finished in 58 seconds (1 minute 26 seconds less than
|
Job | Failed Steps | Tests | |
---|---|---|---|
publish-latest | - 🔗 | N/A | See Details |
publish-stable | - 🔗 | N/A | See Details |
build-dev | - 🔗 | N/A | See Details |
⭕ build-and-test-windows workflow has finished in 5 seconds (40 minutes 30 seconds less than main
branch avg.) and finished at 31st Jan, 2023.
Job | Failed Steps | Tests | |
---|---|---|---|
windows-unittest-matrix | - 🔗 | N/A | See Details |
windows-unittest | - 🔗 | N/A | See Details |
✅ check-links workflow has finished in 1 minute 14 seconds (37 seconds less than main
branch avg.) and finished at 31st Jan, 2023.
Job | Failed Steps | Tests | |
---|---|---|---|
changed files | - 🔗 | N/A | See Details |
check-links | - 🔗 | N/A | See Details |
✅ telemetrygen workflow has finished in 1 minute 34 seconds (50 seconds less than main
branch avg.) and finished at 31st Jan, 2023.
Job | Failed Steps | Tests | |
---|---|---|---|
build-dev | - 🔗 | N/A | See Details |
publish-stable | - 🔗 | N/A | See Details |
publish-latest | - 🔗 | N/A | See Details |
✅ changelog workflow has finished in 2 minutes 32 seconds and finished at 31st Jan, 2023.
Job | Failed Steps | Tests | |
---|---|---|---|
changelog | - 🔗 | N/A | See Details |
✅ prometheus-compliance-tests workflow has finished in 4 minutes 5 seconds (3 minutes 27 seconds less than main
branch avg.) and finished at 31st Jan, 2023.
Job | Failed Steps | Tests | |
---|---|---|---|
prometheus-compliance-tests | - 🔗 | ✅ 21 ❌ 0 ⏭ 0 🔗 | See Details |
✅ build-and-test workflow has finished in 47 minutes 56 seconds and finished at 31st Jan, 2023.
Job | Failed Steps | Tests | |
---|---|---|---|
correctness-metrics | - 🔗 | ✅ 2 ❌ 0 ⏭ 0 🔗 | See Details |
correctness-traces | - 🔗 | ✅ 17 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.18, extension) | - 🔗 | ✅ 546 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, internal) | - 🔗 | ✅ 561 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.18, internal) | - 🔗 | ✅ 561 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.18, processor) | - 🔗 | ✅ 1471 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, extension) | - 🔗 | ✅ 546 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.18, receiver-0) | - 🔗 | ✅ 2573 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, processor) | - 🔗 | ✅ 1471 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, receiver-0) | - 🔗 | ✅ 2573 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.18, exporter) | - 🔗 | ✅ 2421 ❌ 0 ⏭ 0 🔗 | See Details |
integration-tests | - 🔗 | ✅ 55 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, exporter) | - 🔗 | ✅ 2421 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.18, receiver-1) | - 🔗 | ✅ 1950 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, receiver-1) | - 🔗 | ✅ 1950 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.18, other) | - 🔗 | ✅ 4652 ❌ 0 ⏭ 0 🔗 | See Details |
unittest-matrix (1.19, other) | - 🔗 | ✅ 4652 ❌ 0 ⏭ 0 🔗 | See Details |
setup-environment | - 🔗 | N/A | See Details |
check-codeowners | - 🔗 | N/A | See Details |
build-examples | - 🔗 | N/A | See Details |
check-collector-module-version | - 🔗 | 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 |
lint | - 🔗 | N/A | See Details |
unittest (1.19) | - 🔗 | N/A | See Details |
unittest (1.18) | - 🔗 | 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, arm64) | - 🔗 | N/A | See Details |
cross-compile (linux, ppc64le) | - 🔗 | N/A | See Details |
cross-compile (windows, 386) | - 🔗 | N/A | See Details |
cross-compile (windows, amd64) | - 🔗 | 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-stable | - 🔗 | N/A | See Details |
publish-dev | - 🔗 | N/A | See Details |
✅ load-tests workflow has finished in 20 minutes 33 seconds (⚠️ 6 minutes 18 seconds more than main
branch avg.) and finished at 31st Jan, 2023.
Job | Failed Steps | Tests | |
---|---|---|---|
loadtest (TestIdleMode) | - 🔗 | ✅ 1 ❌ 0 ⏭ 0 🔗 | See Details |
loadtest (TestTraceAttributesProcessor) | - 🔗 | ✅ 3 ❌ 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 |
*You can configure Foresight comments in your organization settings page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It feels rather odd to change how errors are reported, going from a static string to a formatted JSON string.
@@ -51,7 +51,7 @@ const ( | |||
responseErrInternalServerError = "Internal Server Error" | |||
responseErrUnsupportedMetricEvent = "Unsupported metric event" | |||
responseErrUnsupportedLogEvent = "Unsupported log event" | |||
|
|||
responseErrHandlingIndexedFields = `{"text":"Error in handling indexed fields","code":15,"invalid-event-number":%d}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any reason to have a JSON formatted string here with substitution?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, splunk returns the message in JSON format that I have used for responseErrHandlingIndexedFields
. And invalid-event-number
points to the index of the event where the problem is.
failRequest
method sets Content-Type
as application/json
but response body is raw text.
resp.Header().Add("Content-Type", "application/json") |
We also need to change other err messages to JSON format with respect to the actual splunk response. I'll create a separate issue for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huh, interesting. Thanks for checking that for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Created #18097
Could I ask you to fix up the linting issues? |
603be18
to
bf758a3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
changes look good to me
bf758a3
to
f7e1b10
Compare
f7e1b10
to
c34d2e4
Compare
Description:
According to the Splunk HEC doc, fields key specifies a JSON object that contains a flat (not nested) list of explicit custom fields to be defined at index time. The current implementation allows receiving nested fields. This PR adds validation for nested fields. When nested fields are found, the entire batch is dropped and will return a 400 error code.
Link to tracking Issue: #17308
Testing: Added necessary unit tests
Documentation: