Skip to content

checkpoint_harmony_endpoint.forensics: Improve error reporting for API requests #12778

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
kcreddy merged 3 commits into from
Feb 14, 2025
Merged

checkpoint_harmony_endpoint.forensics: Improve error reporting for API requests #12778

kcreddy merged 3 commits into from
Feb 14, 2025

Conversation

@kcreddy
Copy link
Contributor

@kcreddy kcreddy commented Feb 13, 2025 •
edited
Loading

Proposed commit message

Improves error reporting for API requests in 
`checkpoint_harmony_endpoint.forensics` data-stream

Note

Better reviewable when Hide-Whitespace is selected.

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.
  • I have verified that any added dashboard complies with Kibana's Dashboard good practices

How to test this PR locally

@kcreddy kcreddy self-assigned this Feb 13, 2025
@kcreddy kcreddy added enhancement New feature or request Team:Security-Service Integrations Security Service Integrations team [elastic/security-service-integrations] Integration:checkpoint_harmony_endpoint Check Point Harmony Endpoint labels Feb 13, 2025
@kcreddy kcreddy marked this pull request as ready for review February 13, 2025 18:36
@kcreddy kcreddy requested a review from a team as a code owner February 13, 2025 18:36
@elasticmachine
Copy link

elasticmachine commented Feb 13, 2025

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

@elastic-vault-github-plugin-prod
Copy link

elastic-vault-github-plugin-prod bot commented Feb 13, 2025

🚀 Benchmarks report

To see the full report comment with /test benchmark fullreport

efd6
efd6 reviewed Feb 13, 2025
View reviewed changes
packages/checkpoint_harmony_endpoint/data_stream/forensics/agent/stream/cel.yml.hbs Outdated
"error": {
"code": string(resp.StatusCode),
"id": string(resp.Status),
"message": "POST " + state.url + "/auth/external" + ": " + (
Copy link
Contributor

@efd6 efd6 Feb 13, 2025

Choose a reason for hiding this comment

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

Suggested change
"message": "POST " + state.url + "/auth/external" + ": " + (
"message": "POST " + state.url.trim_right("/") + "/auth/external: " + (

Copy link
Contributor Author

@kcreddy kcreddy Feb 14, 2025

Choose a reason for hiding this comment

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

Updated as suggested in 3d5cbff

packages/checkpoint_harmony_endpoint/data_stream/forensics/agent/stream/cel.yml.hbs Outdated
"error": {
"code": string(resp.StatusCode),
"id": string(resp.Status),
"message": "POST " + state.url + "/app/laas-logs-api/api/logs_query/retrieve: " + (
Copy link
Contributor

@efd6 efd6 Feb 13, 2025

Choose a reason for hiding this comment

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

Suggested change
"message": "POST " + state.url + "/app/laas-logs-api/api/logs_query/retrieve: " + (
"message": "POST " + state.url.trim_right("/") + "/app/laas-logs-api/api/logs_query/retrieve: " + (

Copy link
Contributor Author

@kcreddy kcreddy Feb 14, 2025

Choose a reason for hiding this comment

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

Updated in 3d5cbff

packages/checkpoint_harmony_endpoint/data_stream/forensics/agent/stream/cel.yml.hbs Outdated
"error": {
"code": string(resp.StatusCode),
"id": string(resp.Status),
"message": "GET " + state.url + "/app/laas-logs-api/api/logs_query/" + state.cursor.task_id + ": " + (
Copy link
Contributor

@efd6 efd6 Feb 13, 2025

Choose a reason for hiding this comment

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

Suggested change
"message": "GET " + state.url + "/app/laas-logs-api/api/logs_query/" + state.cursor.task_id + ": " + (
"message": "GET " + state.url.trim_right("/") + "/app/laas-logs-api/api/logs_query/" + state.cursor.task_id + ": " + (

When I'm doing this, I don't add any details beyond the endpoint, so, I'd also suggest dropping the " + state.cursor.task_id + " part.

Copy link
Contributor Author

@kcreddy kcreddy Feb 14, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Got it, I also thought of removing it, but the path seemed to be same for above error message, hence preventing us from differentiating between errors.

Now I noticed that the http.method is different GET vs POST.

I will remove the state.cursor.task_id + " part.

Copy link
Contributor Author

@kcreddy kcreddy Feb 14, 2025

Choose a reason for hiding this comment

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

Updated in 3d5cbff

@kcreddy kcreddy requested a review from efd6 February 14, 2025 05:21
@elasticmachine
Copy link

elasticmachine commented Feb 14, 2025

💚 Build Succeeded

History

cc @kcreddy

@elastic-sonarqube
Copy link

elastic-sonarqube bot commented Feb 14, 2025

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube

@kcreddy kcreddy merged commit e5af904 into elastic:main Feb 14, 2025
6 checks passed
@elastic-vault-github-plugin-prod
Copy link

elastic-vault-github-plugin-prod bot commented Feb 14, 2025

Package checkpoint_harmony_endpoint - 0.3.0 containing this change is available at https://epr.elastic.co/package/checkpoint_harmony_endpoint/0.3.0/

@kcreddy kcreddy mentioned this pull request Feb 15, 2025
Merged
5 tasks
kcreddy added a commit that referenced this pull request Feb 17, 2025
@kcreddy
checkpoint_harmony_endpoint: Fix indentation in forensics CEL program (…
fbc7d14 
…#12795)

Fix indentation in forensics CEL program caused by #12778
flexitrev pushed a commit that referenced this pull request Mar 20, 2025
@kcreddy @flexitrev
checkpoint_harmony_endpoint.forensics: Improve error reporting for AP…
cb394a4 
…I requests (#12778)

Improves error reporting for API requests in 
`checkpoint_harmony_endpoint.forensics` data-stream
flexitrev pushed a commit that referenced this pull request Mar 20, 2025
@kcreddy @flexitrev
checkpoint_harmony_endpoint: Fix indentation in forensics CEL program (…
bece59e 
…#12795)

Fix indentation in forensics CEL program caused by #12778
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@efd6 efd6 efd6 approved these changes

Assignees

@kcreddy kcreddy

Labels

enhancement New feature or request Integration:checkpoint_harmony_endpoint Check Point Harmony Endpoint 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

@kcreddy @elasticmachine @efd6