Skip to content
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

Include ExtraData from customDetector http response #3385

Open
CameronLonsdale opened this issue Oct 9, 2024 · 2 comments · May be fixed by #3411
Open

Include ExtraData from customDetector http response #3385

CameronLonsdale opened this issue Oct 9, 2024 · 2 comments · May be fixed by #3411

Comments

@CameronLonsdale
Copy link

Please review the Community Note before submitting

Description

When validating a customDetector, include the response body as part of the ExtraData

// TODO: Read response body.

Preferred Solution

Read the response body and include the body as ExtraData

@kashifkhan0771
Copy link
Contributor

kashifkhan0771 commented Oct 14, 2024

I'm considering reading the response body based on the Content-Type header. If the content type is application/json, we can unmarshal it into a JSON structure; if it's text/plain, we can simply convert it to a string. We could also handle other common content types similarly.

@zricethezav @abmussani @ahrav any thoughts?

@ahrav
Copy link
Collaborator

ahrav commented Oct 14, 2024

I'm considering reading the response body based on the Content-Type header. If the content type is application/json, we can unmarshal it into a JSON structure; if it's text/plain, we can simply convert it to a string. We could also handle other common content types similarly.

@zricethezav @abmussani @ahrav any thoughts?

That seems like a good initial approach. Long term, we might consider including it in the custom detector config. However, since we're moving toward a config-based detector setup, there's no need to over-engineer this solution. Your approach is practical for now. 👍

@kashifkhan0771 kashifkhan0771 linked a pull request Oct 15, 2024 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

3 participants