-
Notifications
You must be signed in to change notification settings - Fork 442
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
[infoblox_bloxone_ddi] Initial Release for the Infoblox BloxOne DDI #4118
Conversation
Pinging @elastic/security-external-integrations (Team:Security-External Integrations) |
🌐 Coverage report
|
packages/bloxone_ddi/data_stream/dhcp_lease/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
- name: protocol | ||
type: group | ||
fields: | ||
- name: fqdn |
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.
In the API this is protocol_fqdn
. Is there a need to make a new level for this here?
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.
The only reason why we have kept protocol as an object is to handle new fields that are introduced in the future, we can add it directly as a child to existing protocol objects and fields do not have to be flattened. Moreover, switching from flattened to the nested field would be a breaking change for dashboards, and search queries users have already made. However, if you think this change is required, we can change it.
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.
That seems reasonable. @andrewkroh?
- name: protocol | ||
type: group | ||
fields: | ||
- name: zone | ||
type: keyword | ||
description: Zone FQDN in punycode. |
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.
Similar question here; the original is protocol_zone
.
Also elsewhere.
I'm wondering in general if a more informative name might be better, these fields are always the punycode equivalent of their partner. So perhaps calling them punycode rather than protocol might be better.
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.
We have kept it the same because the end user might be more familiar with the original field name ( protocol_zone ) sent by the vendor, and changing it might confuse the user.
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.
I'm not sure about this. I agree that users my be accustomed to the protocol_zone
name, but that's not what we have here because of the change in name; so now we have ….zone
which I think is also confusing. I don't know what should be done about this.
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.
I agree that the user won’t be familiar with protocol.zone
. However, I believe that transition from protocol_zone
to protocol.zone
would be much easier, instead of the user seeing a completely different name. Let me know if you think otherwise.
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's essentially the same question as above. I'm not strongly opposed to how it is right now, so let's leave it as is unless someone else feels strongly about it.
🚀 Benchmarks reportTo see the full report comment with |
What does this PR do?
NOTE: DNS Data and DHCP Lease data streams are not tested with live data.
Integration release checklist
This checklist is intended for integrations maintainers to ensure consistency
when creating or updating a Package, Module or Dataset for an Integration.
All changes
New Package
Dashboards changes
Log dataset changes
How to test this PR locally
Related issues
Screenshots