[Azure] Sanitize message in case of malformed json #34874

lucian-ioan · 2023-03-21T16:19:53Z

What does this PR do?

This PR adds the function sanitize which is used in parseMultipleMessages to clean up the message string before attempting to unmarshal the JSON.

It also adds the field SanitizeOptions which should be added manually for the datastreams that require sanitization in integrations.

Why is it important?

As pointed out by @zmoog, some logs from Azure can have issues with newlines, while others with single quotes.

This can lead to failure in processing the data in integrations (ex: the pipeline receiving one document with two records).

Filebeat sample configuration

eventhub: ""
connection_string: ""
storage_account: ""
storage_account_key: ""
sanitize_options:
  - NEW_LINES
  - SINGLE_QUOTES

Checklist

I have commented my code, particularly in hard-to-understand areas
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Author's Checklist

Enhance testing with real world examples

Related issues

Closes [azure-evenhub input] fail to process AppServicePlatformLogs due to unescaped newlines #34092

elasticmachine · 2023-03-21T17:37:43Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Start Time: 2023-05-29T11:31:09.436+0000
Duration: 71 min 41 sec

Test stats 🧪

Test	Results
Failed	0
Passed	2920
Skipped	175
Total	3095

💚 Flaky test report

Tests succeeded.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

lucian-ioan · 2023-04-13T13:38:03Z

The function parseMultipleMessages in input.go does not keep the original order of the json keys.

Therefore, the expected result has the json keys in different order (sanitization works fine).

x-pack/filebeat/input/azureeventhub/config.go

x-pack/filebeat/input/azureeventhub/input.go

dmathieu · 2023-04-20T10:14:53Z

x-pack/filebeat/input/azureeventhub/input.go

+	// clean up the message for known issues producing a malformed JSON
+	if a.config.SanitizeOptions != nil {
+		for _, opt := range a.config.SanitizeOptions {
+			bMessage = sanitize(string(bMessage), opt)


What if an unknown/invalid opt is passed here?

My idea is to handle this in the sanitize function. If an unknown option is passed, it should just return the JSON string as it was.

dmathieu · 2023-04-20T10:16:50Z

x-pack/filebeat/input/azureeventhub/sanitization.go

+type sanitizationFunc func(jsonStr string) []byte
+
+func getSanitizationFuncs() map[string]sanitizationFunc {
+	return map[string]sanitizationFunc{


Wouldn't a switch/case be more readable here (it would also allow having a default behavior).

Should the default behaviour be for all sanitizations to occur for Azure?

And how about exposing the sanitization options directly in Kibana for the end-user?

What's your opinion on this? @zmoog

Here's my take:

The default behavior must be no sanitization; these malformed JSON documents are edge cases.

We need to make this option available in the agent template file and on the integration settings page.

For example, we can turn it on by default for FunctionApp logs but also give the option to turn it off as an escape hatch.

This triggers a question: we will probably release this change with stack version 8.8 and backport it to 8.7.2. How can we make this fix available without raising the minimum required version for the Azure Integration logs to 8.7.2? Today the min required version is ^7.16.0 || ^8.0.0.

It's a big bump to address and an edge case.

It seems we must bump the minimum version to 8.9.

x-pack/filebeat/input/azureeventhub/sanitization_test.go

lucian-ioan · 2023-04-20T12:04:21Z

beats/x-pack/filebeat/input/azureeventhub/input.go

Line 171 in b062987

err := json.Unmarshal(bMessage, &mapObject)

Since unmrashalling happens via a map, I think we only need to worry about newlines in the key/values themselves.
(any other newlines are cut during the process)

zmoog · 2023-04-21T15:28:52Z

@lucianpy, please make sure the settings are easy to use in the Agent template and in integration settings UI.

Set this PR ready for review when you're ready.

I will ask around what are our options to make this new configuration settings available to the integration without bumping the min version from 7.17.x to 8.7.2 for an edge case.

zmoog

Hey @lucianpy, how do you plan to set up the integration settings UI to configure the sanitization options?

CHANGELOG.next.asciidoc

x-pack/filebeat/input/azureeventhub/input.go

zmoog · 2023-05-17T15:38:45Z

@lucianpy, we're almost ready to go!

The last thing to try is setting up the new sanitization options in the integration settings UI to ensure we don't need to make any changes to the config struct.

Co-authored-by: Maurizio Branca <[email protected]>

lucian-ioan · 2023-05-18T15:41:31Z

@zmoog I've set up the settings. They will be separate toggles for each sanitization option under 'Advanced options'.
The config under the policy created also looks fine.

zmoog

@lucianpy successfully tested we have everything we need to use this from the Agent integrations settings, so we're good to go!

zmoog · 2023-05-29T13:36:16Z

Hey, @elastic/elastic-agent-data-plane, @lucianpy also needs your review since you are the default owner in the Beats repo.

cmacknz · 2023-05-29T16:36:18Z

Looks like the data plane team is only being pinged because there is no owner set for the azureeventhub input. Rather than requiring us to review, change the codeowner for this input to the team that actually owns it and then I'll approve that. That way we aren't needlessly pinged every time this code changes.

lalit-satapathy · 2023-05-31T04:19:15Z

Looks like the data plane team is only being pinged because there is no owner set for the azureeventhub input. Rather than requiring us to review, change the codeowner for this input to the team that actually owns it and then I'll approve that. That way we aren't needlessly pinged every time this code changes.

Classic example of wrong team being pinged because not-up-to-date CODEOWNER entry and hence delaying the PR review process.

FYI: @andresrc @ruflin

zmoog · 2023-05-31T08:23:27Z

Looks like the data plane team is only being pinged because there is no owner set for the azureeventhub input. Rather than requiring us to review, change the codeowner for this input to the team that actually owns it and then I'll approve that. That way we aren't needlessly pinged every time this code changes.

You are totally right @cmacknz. My apologies for the noise.

I opened a PR to assign the ownership of this input to my team.

* Add sanitization function and test for azure input (cherry picked from commit 4096f9b)

* Add sanitization function and test for azure input

…json (#35622) * [Azure] Sanitize message in case of malformed json (#34874) * Add sanitization function and test for azure input (cherry picked from commit 4096f9b) * Remove extra changelog items from cherry-pick --------- Co-authored-by: lucianpy <[email protected]> Co-authored-by: Maurizio Branca <[email protected]>

… json (#35623) * [Azure] Sanitize message in case of malformed json (#34874) * Add sanitization function and test for azure input (cherry picked from commit 4096f9b) * Fix logger import * fix linter * Fix changelog --------- Co-authored-by: lucianpy <[email protected]> Co-authored-by: lucian-ioan <[email protected]> Co-authored-by: Denis <[email protected]>

Add sanitization function and test for azure input

bd610ec

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Mar 21, 2023

mergify bot assigned lucian-ioan Mar 21, 2023

lucian-ioan requested a review from zmoog March 21, 2023 17:09

lucian-ioan added the Team:Cloud-Monitoring Label for the Cloud Monitoring team label Mar 22, 2023

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Mar 22, 2023

elastic deleted a comment from mergify bot Mar 22, 2023

lucian-ioan mentioned this pull request Mar 28, 2023

[Azure] Add Monitor App Service Logs data stream elastic/integrations#4818

Merged

16 tasks

Add testcase for FunctionAppLogs

c97794c

zmoog reviewed Apr 17, 2023

View reviewed changes

x-pack/filebeat/input/azureeventhub/config.go Outdated Show resolved Hide resolved

Lucian added 2 commits April 19, 2023 03:43

Refactor sanitization using modular approach

5c918de

make update

1e4b628

dmathieu reviewed Apr 20, 2023

View reviewed changes

Lucian added 5 commits April 28, 2023 15:58

validate sanitization options in config

3e41c8a

refactor sanitization logic

9e5a20c

refactor sanitization tests using subtests

18dd2b9

remove redundant logic

3aed9d0

fix small issues

b8fa4ab

lucian-ioan changed the title ~~Sanitize message in case of malformed json~~ [Azure] Sanitize message in case of malformed json Apr 28, 2023

lucian-ioan assigned zmoog and unassigned zmoog May 2, 2023

Lucian and others added 3 commits May 4, 2023 11:05

add docstrigs

ae71f6d

Merge branch 'main' into sanitize_azure_input

e44281e

Add changelog entry

e8fa2c4

lucian-ioan marked this pull request as ready for review May 4, 2023 09:03

Merge remote-tracking branch 'upstream/main' into sanitize_azure_input

a10ad1e

zmoog reviewed May 17, 2023

View reviewed changes

CHANGELOG.next.asciidoc Outdated Show resolved Hide resolved

x-pack/filebeat/input/azureeventhub/input.go Outdated Show resolved Hide resolved

Lucian and others added 4 commits May 18, 2023 07:37

refactor sanitizeSingleQuotes

df08c30

rename jsonStr to jsonByte

e4e4df2

update CHANGELOG.next.asciidoc

67fcc55

Co-authored-by: Maurizio Branca <[email protected]>

update sanitization summary

cbe18cf

Co-authored-by: Maurizio Branca <[email protected]>

zmoog approved these changes May 26, 2023

View reviewed changes

Merge branch 'main' into sanitize_azure_input

967d985

zmoog removed their assignment May 29, 2023

lucian-ioan mentioned this pull request May 30, 2023

[Azure] Integration settings UI for sanitization elastic/integrations#6357

Merged

zmoog merged commit 4096f9b into elastic:main May 31, 2023

zmoog added the backport-v8.8.0 Automated backport with mergify label May 31, 2023

mergify bot pushed a commit that referenced this pull request May 31, 2023

[Azure] Sanitize message in case of malformed json (#34874)

b744964

* Add sanitization function and test for azure input (cherry picked from commit 4096f9b)

mergify bot mentioned this pull request May 31, 2023

[8.8](backport #34874) [Azure] Sanitize message in case of malformed json #35622

Merged

zmoog added the backport-7.17 Automated backport to the 7.17 branch with mergify label May 31, 2023

mergify bot pushed a commit that referenced this pull request May 31, 2023

[Azure] Sanitize message in case of malformed json (#34874)

65ab265

* Add sanitization function and test for azure input (cherry picked from commit 4096f9b)

mergify bot mentioned this pull request May 31, 2023

[7.17](backport #34874) [Azure] Sanitize message in case of malformed json #35623

Merged

chrisberkhout pushed a commit that referenced this pull request Jun 1, 2023

[Azure] Sanitize message in case of malformed json (#34874)

d9d0db0

* Add sanitization function and test for azure input

reakaleek mentioned this pull request Jul 19, 2023

Fix ironbank validation in 8.8 #36115

Closed

6 tasks

devamanv mentioned this pull request Aug 11, 2023

Add Metrics data stream for Azure Functions elastic/integrations#6704

Closed

2 tasks

lucian-ioan mentioned this pull request May 24, 2024

[Azure] [OpenAI] Malformed FunctionAppLogs documents found in azure_openai.logs dataset elastic/integrations#9964

Closed

zmoog added the input:azure-eventhub label Aug 13, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Azure] Sanitize message in case of malformed json #34874

[Azure] Sanitize message in case of malformed json #34874

lucian-ioan commented Mar 21, 2023 •

edited

Loading

elasticmachine commented Mar 21, 2023 •

edited by jenkins-beats-ci bot

Loading

Build stats

Test stats 🧪

lucian-ioan commented Apr 13, 2023

dmathieu Apr 20, 2023

lucian-ioan Apr 20, 2023 •

edited

Loading

dmathieu Apr 20, 2023

lucian-ioan Apr 20, 2023 •

edited

Loading

zmoog Apr 21, 2023 •

edited

Loading

zmoog May 17, 2023

lucian-ioan commented Apr 20, 2023 •

edited

Loading

zmoog commented Apr 21, 2023

zmoog left a comment

zmoog commented May 17, 2023

lucian-ioan commented May 18, 2023 •

edited

Loading

zmoog left a comment

zmoog commented May 29, 2023

cmacknz commented May 29, 2023

lalit-satapathy commented May 31, 2023

zmoog commented May 31, 2023

[Azure] Sanitize message in case of malformed json #34874

[Azure] Sanitize message in case of malformed json #34874

Conversation

lucian-ioan commented Mar 21, 2023 • edited Loading

What does this PR do?

Why is it important?

Filebeat sample configuration

Checklist

Author's Checklist

Related issues

elasticmachine commented Mar 21, 2023 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

Test stats 🧪

💚 Flaky test report

🤖 GitHub comments

lucian-ioan commented Apr 13, 2023

dmathieu Apr 20, 2023

Choose a reason for hiding this comment

lucian-ioan Apr 20, 2023 • edited Loading

Choose a reason for hiding this comment

dmathieu Apr 20, 2023

Choose a reason for hiding this comment

lucian-ioan Apr 20, 2023 • edited Loading

Choose a reason for hiding this comment

zmoog Apr 21, 2023 • edited Loading

Choose a reason for hiding this comment

zmoog May 17, 2023

Choose a reason for hiding this comment

lucian-ioan commented Apr 20, 2023 • edited Loading

zmoog commented Apr 21, 2023

zmoog left a comment

Choose a reason for hiding this comment

zmoog commented May 17, 2023

lucian-ioan commented May 18, 2023 • edited Loading

zmoog left a comment

Choose a reason for hiding this comment

zmoog commented May 29, 2023

cmacknz commented May 29, 2023

lalit-satapathy commented May 31, 2023

zmoog commented May 31, 2023

lucian-ioan commented Mar 21, 2023 •

edited

Loading

elasticmachine commented Mar 21, 2023 •

edited by jenkins-beats-ci bot

Loading

lucian-ioan Apr 20, 2023 •

edited

Loading

lucian-ioan Apr 20, 2023 •

edited

Loading

zmoog Apr 21, 2023 •

edited

Loading

lucian-ioan commented Apr 20, 2023 •

edited

Loading

lucian-ioan commented May 18, 2023 •

edited

Loading