Mapping of Gigamon Metadata Attributes to ECS fields#14692
Mapping of Gigamon Metadata Attributes to ECS fields#14692efd6 merged 2 commits intoelastic:mainfrom
Conversation
31da13b to
9adc056
Compare
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
|
/test |
efd6
left a comment
There was a problem hiding this comment.
Please use the PR template. Can you provide a proposed commit message to repair that?
| type: keyword | ||
| - name: http_rtt | ||
| type: keyword | ||
| type: float |
There was a problem hiding this comment.
This is not OK. Please revert this.
There was a problem hiding this comment.
For a recent use case from customer, we need to perform math operation for this attribute. So, changed the data type from keyword to float
There was a problem hiding this comment.
Doing this is a breaking change.
There was a problem hiding this comment.
@apps-elastic-gigamon, any datatype changes not within the same family, for example: keyword family, is a breaking-change because it leads to mapping conflicts for existing users.
If this is must, you will need to update changelog version as 2.0.0 with type: breaking-change. Example:
integrations/packages/ti_abusech/changelog.yml
Lines 11 to 17 in ed4eea9
There was a problem hiding this comment.
Code changes done as per the comments.
- version: "2.0.0"
changes:- description: Update mapping of Gigamon attributes to align with ECS fields
type: breaking-change
link: Mapping of Gigamon Metadata Attributes to ECS fields #14692
- description: Update mapping of Gigamon attributes to align with ECS fields
There was a problem hiding this comment.
Rather than mutating existing tests, please add new tests that cover the additions here.
There was a problem hiding this comment.
Added new JSON records as per the comments
packages/gigamon/data_stream/ami/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
packages/gigamon/data_stream/ami/elasticsearch/ingest_pipeline/default.yml
Outdated
Show resolved
Hide resolved
packages/gigamon/changelog.yml
Outdated
| # newer versions go on top | ||
| - version: "1.7.0" | ||
| changes: | ||
| - description: Mapping of Gigamon Attributes to ECS fields |
There was a problem hiding this comment.
This should be a sentence; suggest "Improve mapping of Gigamon attributes to ECS fields."
There was a problem hiding this comment.
We are currently mapping the Gigamon attributes with the appropriate ECS fields, rather than making enhancements to the existing mappings.
There was a problem hiding this comment.
The current text is not a valid English sentence since it has no verb. Please apply the change or something equivalent. The suggested change adds an appropriate verb since this is an improvement.
There was a problem hiding this comment.
Changed the description as per comments
| type: long | ||
| - name: app_id | ||
| type: long | ||
| type: keyword |
There was a problem hiding this comment.
This is unfortunate, but cannot be changed without a breaking change. Please revert this.
There was a problem hiding this comment.
We have mapped service.id ECS field to this app_id. Hence the data type has to be changed. Also, in the backend we haven't done any math operations using this app_id
There was a problem hiding this comment.
Yes, it's unfortunate that the field was originally defined as a long rather than a keyword as is conventional for an ID field, but changing this from a long to a keyword is a breaking change, so making such a change would require a significant benefit to counter the cost of breaking people.
It does need to be a keyword when it is copied to event.id, but that can either be done as a convert to a string, or just by a copy since numeric values can be mapped as keywords.
There was a problem hiding this comment.
Updated the changelog.yml accordingly
There was a problem hiding this comment.
I still do not believe this change is justified.
kcreddy
left a comment
There was a problem hiding this comment.
CI is failing on error: Error: checking package failed: checking readme files are up-to-date failed: files do not match
To fix this CI error, you will need to run elastic-package build and commit the generated docs/README.md.
77c9706 to
e65845c
Compare
|
/test |
packages/gigamon/data_stream/ami/elasticsearch/ingest_pipeline/default.yml
Show resolved
Hide resolved
🚀 Benchmarks reportPackage
|
| Data stream | Previous EPS | New EPS | Diff (%) | Result |
|---|---|---|---|---|
ami |
4694.84 | 2890.17 | -1804.67 (-38.44%) | 💔 |
To see the full report comment with /test benchmark fullreport
e65845c to
349ef32
Compare
|
/test |
| - version: "2.0.0" | ||
| changes: | ||
| - description: Update mapping of Gigamon attributes to align with ECS fields | ||
| type: breaking-change |
There was a problem hiding this comment.
I do not believe the balance of benefits/costs supports a breaking change here.
| type: long | ||
| - name: app_id | ||
| type: long | ||
| type: keyword |
There was a problem hiding this comment.
I still do not believe this change is justified.
|
/test |
💚 Build Succeeded
History
|
|
|
Package gigamon - 2.0.0 containing this change is available at https://epr.elastic.co/package/gigamon/2.0.0/ |
…#14692) This is an enhancement change for the mapping of Gigamon Metadata Attributes to corresponding ECS fields.




Proposed commit message :
This is an enhancement change for the mapping of Gigamon Metadata Attributes to corresponding ECS fields
Checklist
changelog.ymlfile.Related issues