[axonius][storage] Add Axonius Storage datastream#16605
Conversation
muskan-agarwal26
changed the title
andrewkroh
added
Integration:axonius
|
Hi! We just realized that we haven't looked into this PR in a while. We're sorry! We're labeling this issue as |
|
Hi! This PR has been stale for a while and we're going to close it as part of our cleanup procedure. We appreciate your contribution and would like to apologize if we have not been able to review it, due to the current heavy load of the team. Feel free to re-open this PR if you think it should stay open and is worth rebasing. Thank you for your contribution! |
muskan-agarwal26
force-pushed
the
data_stream-storage
branch
from
daf3add to
615bf93
Compare
ShourieG
left a comment
ShourieG
left a comment
There was a problem hiding this comment.
🤖 AI-Generated Review | Elastic Integration PR Review Bot
⚠️ This is an automated review generated by an AI assistant. Please verify all suggestions before applying changes. This review does not represent a human reviewer's opinion.
PR Review | elastic/integrations #16605
Field Mapping
Data Stream: storage (package: axonius)
File: packages/axonius/data_stream/storage/fields/fields.yml
Issue 1: All custom fields are missing description properties
Severity: 🟡 Medium
Location: packages/axonius/data_stream/storage/fields/fields.yml line 7
Problem: Every leaf field definition under axonius.storage.* is missing a required description property. Every custom field definition must include a human-readable description explaining what the field contains.
Recommendation:
- name: adapter_list_length
type: long
description: Number of adapters associated with this asset.
- name: adapters
type: keyword
description: List of adapter names that discovered this asset.
- name: asset_type
type: keyword
description: Classification type of the asset.Issue 2: Field size uses double — verify if long is more appropriate
Severity: 🔵 Low
Location: packages/axonius/data_stream/storage/fields/fields.yml line 65
Problem: axonius.storage.size uses type: double. If this field represents a storage size in bytes or an integer count, long is the more appropriate type. double should only be used when fractional/decimal values are expected.
Recommendation:
- name: size
type: long
description: Storage size of the asset in bytes.💡 Suggestions
- Consider adding
example:values to complex fields likeadapters,urls,quick_id, andtransform_unique_idto aid documentation and onboarding. - The field
axonius.storage.eventcreates a naming collision risk with the ECSevent.*namespace. While nested underaxonius.storage, confirm this sub-group is intentional and does not conflict with pipeline-set ECSevent.*fields. tenant_numberusestype: keyword— if this is always a numeric value, consider whetherlongis more appropriate.urlsusestype: keyword— if multiple URLs are stored per document, consider whethermulti_fieldswith atextsub-field would be useful for full-text search.
Pipeline
Data Stream: storage (package: axonius)
File: packages/axonius/data_stream/storage/elasticsearch/ingest_pipeline/default.yml
Issue 1: Script processor for field renaming lacks on_failure handler
Severity: 🟡 Medium
Location: packages/axonius/data_stream/storage/elasticsearch/ingest_pipeline/default.yml line 63
Problem: The script processor (script_rename_event_data_fields) performs complex map manipulation with key conflict resolution but has no on_failure block. A runtime exception (e.g., unexpected data type in axonius.storage.event.data) will propagate to the global handler without a specific error message identifying this processor as the source, making debugging harder.
Recommendation:
- script:
tag: script_rename_event_data_fields
lang: painless
description: Renames all event.data.* fields to root level...
source: |-
...
on_failure:
- append:
field: error.message
value: 'Processor {{{_ingest.on_failure_processor_type}}} with tag {{{_ingest.on_failure_processor_tag}}} in pipeline {{{_ingest.on_failure_pipeline}}} failed with message: {{{_ingest.on_failure_message}}}'Issue 2: Global on_failure handler sets error.message before event.kind
Severity: 🔵 Low
Location: packages/axonius/data_stream/storage/elasticsearch/ingest_pipeline/default.yml line 254
Problem: The global on_failure handler appends to error.message first (line 255) but then sets event.kind: pipeline_error (line 261). The standard pattern sets event.kind first, then appends error.message. Functionally equivalent but inconsistent with the established convention.
Recommendation:
on_failure:
- set:
field: event.kind
tag: set_pipeline_error_to_event_kind
value: pipeline_error
- append:
field: error.message
value: |-
Processor '{{{ _ingest.on_failure_processor_type }}}'
...
- append:
field: tags
value: preserve_original_event
allow_duplicates: false💡 Suggestions
event.categoryandevent.typeare not set. For storage asset data (disks, file systems, object storage), consider addingevent.category: [host]orevent.category: [file]andevent.type: [info]to improve ECS categorization and SIEM correlation.related.hosts/related.ipnot populated — if any storage asset fields contain hostnames or IP addresses (e.g., from adapter data), consider populating these for cross-event correlation.- Verify that
axonius.storage.accurate_for_datetimeat the root level is not a duplicate ofaxonius.storage.event.accurate_for_datetime(which IS removed in cleanup). If it is a duplicate, add it to the cleanupremoveprocessor. - Verify that
axonius.storage.transform_unique_id(written by thefingerprintprocessor at line 48) is declared infields.ymlto avoid dynamic mapping surprises.
Input Configuration
Data Stream: storage (package: axonius)
File: packages/axonius/data_stream/storage/agent/stream/cel.yml.hbs
Issue 1: Manifest variable asset_type_list is declared but never used in CEL template
Severity: 🟠 High
Location: packages/axonius/data_stream/storage/agent/stream/cel.yml.hbs line 22
Problem: The data stream manifest.yml declares asset_type_list as a user-configurable variable with a default of [object_storages, file_systems, disks], but the CEL template hardcodes this list directly in the state: block. Any user customization of this variable in the Fleet UI will be silently ignored.
Recommendation:
state:
api_key: {{api_key}}
secret_key: {{secret_key}}
batch_size: {{batch_size}}
asset_type_list: {{asset_type_list}}Issue 2: next_page cursor may store optional wrapper instead of raw string
Severity: 🟡 Medium
Location: packages/axonius/data_stream/storage/agent/stream/cel.yml.hbs line 88
Problem: The next_page cursor is stored as body.?meta.?next_page, which is a CEL optional wrapper rather than the raw string value. When read back from state and used in the request body via ?"next_page": state.?worklist.?next_page, the optional chaining may not correctly unwrap the stored optional, potentially sending a malformed cursor token to the API on the second page.
Recommendation:
"next_page": (has(body.meta.page.number) && has(body.meta.page.totalPages) &&
int(body.meta.page.number) < int(body.meta.page.totalPages)) ? body.?meta.?next_page.orValue(null) : null,
Issue 3: has(body.meta.page.number) guard does not protect against absent body.meta
Severity: 🔵 Low
Location: packages/axonius/data_stream/storage/agent/stream/cel.yml.hbs line 85
Problem: The has() check on body.meta.page.number does not use optional chaining. If body.meta itself is absent, this will throw a runtime error.
Recommendation:
has(body.?meta.?page.number)
// or
body.?meta.?page.?number.hasValue()
Issue 4: want_more operator precedence ambiguity — missing parentheses around || operands
Severity: 🔵 Low
Location: packages/axonius/data_stream/storage/agent/stream/cel.yml.hbs line 90
Problem: The want_more expression mixes && and || without explicit grouping around the || operands. While CEL's operator precedence makes the current behavior correct, the intent is ambiguous to readers.
Recommendation:
"want_more": (
has(body.meta.page.number) && has(body.meta.page.totalPages) &&
int(body.meta.page.number) < int(body.meta.page.totalPages)
) || size(state.worklist.asset_type_list) > 1,
Issue 5: Spurious space in error message URL construction
Severity: 🔵 Low
Location: packages/axonius/data_stream/storage/agent/stream/cel.yml.hbs line 100
Problem: The string "/api/v2/assets/ " contains a trailing space before the closing quote, producing a malformed URL in the error message: POST:https://host/api/v2/assets/ object_storages.
Recommendation:
"POST:" + state.url.trim_right("/") + "/api/v2/assets/" + string(state.worklist.asset_type_list[0]) + ...
Issue 6: CEL program does not match celfmt canonical formatting
Severity: 🔵 Low
Location: packages/axonius/data_stream/storage/agent/stream/cel.yml.hbs line 31
Problem: The CEL program compiles successfully but does not match celfmt canonical formatting. Issues include: missing trailing comma in the inner worklist map literal, inconsistent indentation in the .as(state, ...) chain, and missing parentheses around boolean conditions in ternary expressions.
Recommendation:
Run celfmt on the CEL program and apply the output. See the formatting diff in the raw review for the full set of changes required.
💡 Suggestions
- Verify that the package-level
manifest.ymlspecifieskibana.version: "^8.15.0"or higher — the new-formatresource.tracer:block requires Elastic Agent ≥ 8.15.0, and optional types (.?) require ≥ 8.12.0. - Consider adding explicit HTTP 429 (Too Many Requests) handling. The current generic error path will emit an error event and stop; explicit 429 detection with a backoff hint would be more robust.
Transform
Package: axonius
File: packages/axonius/elasticsearch/transform/latest_storage/transform.yml
Issue 1: Transform frequency below 1m minimum (30s)
Severity: 🟡 Medium
Location: packages/axonius/elasticsearch/transform/latest_storage/transform.yml line 17
Problem: frequency: 30s is below the recommended minimum of 1m. Sub-minute frequencies cause excessive cluster load and are not recommended for production transforms.
Recommendation:
frequency: 5mIssue 2: retention_policy uses event.ingested instead of @timestamp
Severity: 🟡 Medium
Location: packages/axonius/elasticsearch/transform/latest_storage/transform.yml line 30
Problem: The retention_policy.time.field is set to event.ingested rather than @timestamp. Retention policies are conventionally applied against event time (@timestamp), not ingestion time. Using event.ingested means documents are retained/expired based on when they were indexed, not when the storage event occurred, which can produce unexpected results if there is any ingestion lag.
Recommendation:
retention_policy:
time:
field: "@timestamp"
max_age: 24hIssue 3: _meta.managed set to false
Severity: 🔵 Low
Location: packages/axonius/elasticsearch/transform/latest_storage/transform.yml line 33
Problem: _meta.managed is set to false. For Fleet-managed transforms, this should typically be true so Fleet correctly tracks and manages the transform lifecycle.
Recommendation:
_meta:
managed: true
fleet_transform_version: 0.1.0Summary
| Severity | Count |
|---|---|
| 🔴 Critical | 0 |
| 🟠 High | 1 |
| 🟡 Medium | 6 |
| 🔵 Low | 8 |
Total Actionable Items: 15
| - name: storage | ||
| type: group | ||
| fields: | ||
| - name: adapter_list_length |
There was a problem hiding this comment.
🤖 AI Review: 🟡 All custom fields are missing description properties — Every leaf field definition under axonius.storage.* is missing a required description property. Every custom field definition must include a human-readable description explaining what the field contains.
Recommendation:
- name: adapter_list_length
type: long
description: Number of adapters associated with this asset.
- name: adapters
type: keyword
description: List of adapter names that discovered this asset.
- name: asset_type
type: keyword
description: Classification type of the asset.🤖 AI-generated comment by Elastic Integration PR Review Bot
| - name: not_fetched_count | ||
| type: long | ||
| - name: size | ||
| type: double |
There was a problem hiding this comment.
🤖 AI Review: 🟡 Field size uses double — verify if long is more appropriate — axonius.storage.size uses type: double. If this field represents a storage size in bytes or an integer count, long is the more appropriate type. double should only be used when fractional/decimal values are expected.
Recommendation:
- name: size
type: long
description: Storage size of the asset in bytes.💡 Suggestions
- Consider adding
example:values to complex fields likeadapters,urls,quick_id, andtransform_unique_idto aid documentation and onboarding. - The field
axonius.storage.eventcreates a naming collision risk with the ECSevent.*namespace. While nested underaxonius.storage, confirm this sub-group is intentional and does not conflict with pipeline-set ECSevent.*fields. tenant_numberusestype: keyword— if this is always a numeric value, consider whetherlongis more appropriate.urlsusestype: keyword— if multiple URLs are stored per document, consider whethermulti_fieldswith atextsub-field would be useful for full-text search.
🤖 AI-generated comment by Elastic Integration PR Review Bot
| - append: | ||
| field: error.message | ||
| value: 'Processor {{{_ingest.on_failure_processor_type}}} with tag {{{_ingest.on_failure_processor_tag}}} in pipeline {{{_ingest.on_failure_pipeline}}} failed with message: {{{_ingest.on_failure_message}}}' | ||
| - script: |
There was a problem hiding this comment.
🤖 AI Review: 🟡 Script processor for field renaming lacks on_failure handler — The script processor (script_rename_event_data_fields) performs complex map manipulation with key conflict resolution but has no on_failure block. A runtime exception (e.g., unexpected data type in axonius.storage.event.data) will propagate to the global handler without a specific error message identifying this processor as the source, making debugging harder.
Recommendation:
- script:
tag: script_rename_event_data_fields
lang: painless
description: Renames all event.data.* fields to root level...
source: |-
...
on_failure:
- append:
field: error.message
value: 'Processor {{{_ingest.on_failure_processor_type}}} with tag {{{_ingest.on_failure_processor_tag}}} in pipeline {{{_ingest.on_failure_pipeline}}} failed with message: {{{_ingest.on_failure_message}}}'🤖 AI-generated comment by Elastic Integration PR Review Bot
| value: preserve_original_event | ||
| allow_duplicates: false | ||
| if: ctx.error?.message != null | ||
| on_failure: |
There was a problem hiding this comment.
🤖 AI Review: 💡 Global on_failure handler sets error.message before event.kind — The global on_failure handler appends to error.message first (line 255) but then sets event.kind: pipeline_error (line 261). The standard pattern sets event.kind first, then appends error.message. Functionally equivalent but inconsistent with the established convention.
Recommendation:
on_failure:
- set:
field: event.kind
tag: set_pipeline_error_to_event_kind
value: pipeline_error
- append:
field: error.message
value: |-
Processor '{{{ _ingest.on_failure_processor_type }}}'
...
- append:
field: tags
value: preserve_original_event
allow_duplicates: false💡 Suggestions
event.categoryandevent.typeare not set. For storage asset data (disks, file systems, object storage), consider addingevent.category: [host]orevent.category: [file]andevent.type: [info]to improve ECS categorization and SIEM correlation.related.hosts/related.ipnot populated — if any storage asset fields contain hostnames or IP addresses (e.g., from adapter data), consider populating these for cross-event correlation.- Verify that
axonius.storage.accurate_for_datetimeat the root level is not a duplicate ofaxonius.storage.event.accurate_for_datetime(which IS removed in cleanup). If it is a duplicate, add it to the cleanupremoveprocessor. - Verify that `axo
🤖 AI-generated comment by Elastic Integration PR Review Bot
| api_key: {{api_key}} | ||
| secret_key: {{secret_key}} | ||
| batch_size: {{batch_size}} | ||
| asset_type_list: |
There was a problem hiding this comment.
🤖 AI Review: 🔴 Manifest variable asset_type_list is declared but never used in CEL template — The data stream manifest.yml declares asset_type_list as a user-configurable variable with a default of [object_storages, file_systems, disks], but the CEL template hardcodes this list directly in the state: block. Any user customization of this variable in the Fleet UI will be silently ignored.
Recommendation:
state:
api_key: {{api_key}}
secret_key: {{secret_key}}
batch_size: {{batch_size}}
asset_type_list: {{asset_type_list}}🤖 AI-generated comment by Elastic Integration PR Review Bot
| "error": { | ||
| "code": string(resp.StatusCode), | ||
| "id": string(resp.Status), | ||
| "message": "POST:" + state.url.trim_right("/") + "/api/v2/assets/ " + string(state.worklist.asset_type_list[0]) + ( |
There was a problem hiding this comment.
🤖 AI Review: 💡 Spurious space in error message URL construction — The string "/api/v2/assets/ " contains a trailing space before the closing quote, producing a malformed URL in the error message: POST:https://host/api/v2/assets/ object_storages.
Recommendation:
"POST:" + state.url.trim_right("/") + "/api/v2/assets/" + string(state.worklist.asset_type_list[0]) + ...
🤖 AI-generated comment by Elastic Integration PR Review Bot
| fields: | ||
| - api_key | ||
| - secret_key | ||
| program: | |
There was a problem hiding this comment.
🤖 AI Review: 💡 CEL program does not match celfmt canonical formatting — The CEL program compiles successfully but does not match celfmt canonical formatting. Issues include: missing trailing comma in the inner worklist map literal, inconsistent indentation in the .as(state, ...) chain, and missing parentheses around boolean conditions in ternary expressions.
Recommendation:
Run celfmt on the CEL program and apply the output. See the formatting diff in the raw review for the full set of changes required.
💡 Suggestions
- Verify that the package-level
manifest.ymlspecifieskibana.version: "^8.15.0"or higher — the new-formatresource.tracer:block requires Elastic Agent ≥ 8.15.0, and optional types (.?) require ≥ 8.12.0. - Consider adding explicit HTTP 429 (Too Many Requests) handling. The current generic error path will emit an error event and stop; explicit 429 detection with a backoff hint would be more robust.
🤖 AI-generated comment by Elastic Integration PR Review Bot
| sort: "@timestamp" | ||
| description: >- | ||
| Latest storages from Axonius. As storages get updated, this transform stores only the latest state of each storage inside the destination index. Thus the transform's destination index contains only the latest state of the storage. | ||
| frequency: 30s |
There was a problem hiding this comment.
🤖 AI Review: 🟡 Transform frequency below 1m minimum (30s) — frequency: 30s is below the recommended minimum of 1m. Sub-minute frequencies cause excessive cluster load and are not recommended for production transforms.
Recommendation:
frequency: 5m🤖 AI-generated comment by Elastic Integration PR Review Bot
| delay: 120s | ||
| retention_policy: | ||
| time: | ||
| field: "event.ingested" |
There was a problem hiding this comment.
🤖 AI Review: 🟡 retention_policy uses event.ingested instead of @timestamp — The retention_policy.time.field is set to event.ingested rather than @timestamp. Retention policies are conventionally applied against event time (@timestamp), not ingestion time. Using event.ingested means documents are retained/expired based on when they were indexed, not when the storage event occurred, which can produce unexpected results if there is any ingestion lag.
Recommendation:
retention_policy:
time:
field: "@timestamp"
max_age: 24h🤖 AI-generated comment by Elastic Integration PR Review Bot
| field: "event.ingested" | ||
| max_age: 24h | ||
| _meta: | ||
| managed: false |
There was a problem hiding this comment.
🤖 AI Review: 🟡 _meta.managed set to false — _meta.managed is set to false. For Fleet-managed transforms, this should typically be true so Fleet correctly tracks and manages the transform lifecycle.
Recommendation:
_meta:
managed: true
fleet_transform_version: 0.1.0🤖 AI-generated comment by Elastic Integration PR Review Bot
ShourieG
added
the
Team:Security-Service Integrations
|
Pinging @elastic/security-service-integrations (Team:Security-Service Integrations) |
|
@ShourieG , I have resolved the comments from AI, could you please re-review the PR? |
|
/test |
🚀 Benchmarks reportTo see the full report comment with |
💚 Build Succeeded
|
andrewkroh
removed
the
New Integration
|
There are two unaddressed concerns. |
|
@efd6 , I have resolved all the comments, kindly re-review. |
|
Package axonius - 0.1.0 containing this change is available at https://epr.elastic.co/package/axonius/0.1.0/ |
7 participants
Proposed commit message
The release includes storage data stream and associated dashboard.
Axonius fields are mapped to their corresponding ECS fields where possible.
Test samples were derived from live data samples, which were subsequently
sanitized.
Checklist
changelog.ymlfile.How to test this PR locally
To test the axonius package:
Related issues
Screenshots