Skip to content

all: harmonise http status error return behaviour #10346

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 5, 2024
Merged

all: harmonise http status error return behaviour #10346

merged 1 commit into from
Jul 5, 2024

Conversation

efd6
Copy link
Contributor

@efd6 efd6 commented Jul 3, 2024 •
edited
Loading

Proposed commit message

Return a single object with a guaranteed non-empty message when a non-successful status is returned by an API request. Prefix the error message with the HTTP method used to get the response.

A single object is used since it will result in a degraded status being raised by elastic agent.

  • carbon_black_cloud
  • crowdstrike
  • qualys_vmdr
  • snyk
  • symantec_edr_cloud
  • tenable_io
  • ti_abusech
  • ti_crowdstrike
  • ti_threatconnect
  • wiz

Also fix an incorrect test in ti_crowdstrike that was exposed by the presence of a present but empty error message.

  • cybereason is not changed since there are not system tests to ensure that CEL type inference will not break the input
  • tenable_io.{asset,vulnerability} are not changed since there are other more complicated things going on

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Author's Checklist

  • [ ]

How to test this PR locally

Related issues

Screenshots

@efd6 efd6 added enhancement New feature or request Integration:snyk Snyk Integration:crowdstrike CrowdStrike Integration:ti_abusech abuse.ch Integration:carbon_black_cloud VMware Carbon Black Cloud Integration:wiz Wiz Integration:qualys_vmdr Qualys VMDR Integration:ti_crowdstrike CrowdStrike Falcon Intelligence Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations] Integration:ti_threatconnect ThreatConnect (Partner supported) Integration:symantec_edr_cloud Symantec EDR Cloud (Deprecated) [Integration not found in source] labels Jul 3, 2024
@efd6 efd6 self-assigned this Jul 3, 2024
@efd6 efd6 force-pushed the cel_errors_harmonise branch from d731526 to c6d42ca Compare July 3, 2024 06:51
@efd6 efd6 added the Integration:tenable_io Tenable Vulnerability Management label Jul 3, 2024
@elasticmachine
Copy link

elasticmachine commented Jul 3, 2024 •
edited
Loading

🚀 Benchmarks report

Package carbon_black_cloud 👍(5) 💚(0) 💔(1)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
watchlist_hit 3759.4 2732.24 -1027.16 (-27.32%) 💔

Package crowdstrike 👍(3) 💚(0) 💔(1)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
falcon 14084.51 10101.01 -3983.5 (-28.28%) 💔

Package qualys_vmdr 👍(2) 💚(0) 💔(1)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
user_activity 7194.24 5882.35 -1311.89 (-18.24%) 💔

Package ti_abusech 👍(1) 💚(1) 💔(2)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
malwarebazaar 3389.83 2469.14 -920.69 (-27.16%) 💔
url 5208.33 3322.26 -1886.07 (-36.21%) 💔

Package ti_crowdstrike 👍(1) 💚(0) 💔(1)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
ioc 5617.98 3278.69 -2339.29 (-41.64%) 💔

Package wiz 👍(2) 💚(0) 💔(1)

Expand to view
Data stream Previous EPS New EPS Diff (%) Result
issue 2564.1 1584.79 -979.31 (-38.19%) 💔

To see the full report comment with /test benchmark fullreport

@efd6 efd6 force-pushed the cel_errors_harmonise branch from c6d42ca to edf5db7 Compare July 3, 2024 07:20
@efd6 efd6 marked this pull request as ready for review July 3, 2024 07:55
@efd6 efd6 requested a review from a team as a code owner July 3, 2024 07:55
@elasticmachine
Copy link

Pinging @elastic/security-service-integrations (Team:Security-Service Integrations)

@efd6 efd6 force-pushed the cel_errors_harmonise branch from edf5db7 to c8fb49a Compare July 3, 2024 21:57
@efd6
Copy link
Contributor Author

efd6 commented Jul 3, 2024

/test

@efd6 efd6 mentioned this pull request Jul 4, 2024
Closed
@efd6
all: harmonise http status error return behaviour
eb1bd6a 
Return a single object with a guaranteed non-empty message when a
non-successful status is returned by an API request. Prefix the error message
with the HTTP method used to get the response.

A single object is used since it will result in a degraded status being raised
by elastic agent.

* carbon_black_cloud
* crowdstrike
* qualys_vmdr
* snyk
* symantec_edr_cloud
* tenable_io
* ti_abusech
* ti_crowdstrike
* ti_threatconnect
* wiz

Also fix an incorrect test in ti_crowdstrike that was exposed by the presence
of an non-empty error message.
@efd6 efd6 force-pushed the cel_errors_harmonise branch from c8fb49a to eb1bd6a Compare July 5, 2024 00:24
@elasticmachine
Copy link

💚 Build Succeeded

History

  • 💚 Build #13214 succeeded c8fb49ae813314092cfb5ec581d11437a832e996
  • 💔 Build #13211 failed c8fb49ae813314092cfb5ec581d11437a832e996
  • 💚 Build #13166 succeeded edf5db72495b4597b473cf10f461c036e55ba8b3
  • 💔 Build #13162 failed c6d42ca91e23116424427a503d229dfdcc74bc2c
  • 💔 Build #13161 failed d7315268323360ebc88400335e976e5d93109613

cc @efd6

@elastic-sonarqube Elastic SonarQube
Copy link

Quality Gate failed Quality Gate failed

Failed conditions
16.7% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube

@efd6 efd6 merged commit 0c17c7a into elastic:main Jul 5, 2024
@elasticmachine
Copy link

Package carbon_black_cloud - 2.3.0 containing this change is available at https://epr.elastic.co/search?package=carbon_black_cloud

@elasticmachine
Copy link

Package crowdstrike - 1.38.0 containing this change is available at https://epr.elastic.co/search?package=crowdstrike

@elasticmachine
Copy link

Package qualys_vmdr - 3.4.0 containing this change is available at https://epr.elastic.co/search?package=qualys_vmdr

@elasticmachine
Copy link

Package snyk - 1.24.0 containing this change is available at https://epr.elastic.co/search?package=snyk

@elasticmachine
Copy link

Package symantec_edr_cloud - 1.4.0 containing this change is available at https://epr.elastic.co/search?package=symantec_edr_cloud

@elasticmachine
Copy link

Package tenable_io - 3.2.0 containing this change is available at https://epr.elastic.co/search?package=tenable_io

@elasticmachine
Copy link

Package ti_abusech - 2.3.0 containing this change is available at https://epr.elastic.co/search?package=ti_abusech

@elasticmachine
Copy link

Package ti_threatconnect - 1.2.0 containing this change is available at https://epr.elastic.co/search?package=ti_threatconnect

@elasticmachine
Copy link

Package wiz - 1.4.0 containing this change is available at https://epr.elastic.co/search?package=wiz

@efd6 efd6 deleted the cel_errors_harmonise branch February 5, 2025 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ShourieG ShourieG ShourieG approved these changes

Assignees

@efd6 efd6

Labels
enhancement New feature or request Integration:carbon_black_cloud VMware Carbon Black Cloud Integration:crowdstrike CrowdStrike Integration:qualys_vmdr Qualys VMDR Integration:snyk Snyk Integration:symantec_edr_cloud Symantec EDR Cloud (Deprecated) [Integration not found in source] Integration:tenable_io Tenable Vulnerability Management Integration:ti_abusech abuse.ch Integration:ti_crowdstrike CrowdStrike Falcon Intelligence Integration:ti_threatconnect ThreatConnect (Partner supported) Integration:wiz Wiz Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations]
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@efd6 @elasticmachine @ShourieG