Skip to content

fix: map __ts_timeline_id to timeline name in to_pandas - #3895

Open
SkxOverKill wants to merge 2 commits into
google:masterfrom
SkxOverKill:fix/to_pandas-timeline-id-field
Open

fix: map __ts_timeline_id to timeline name in to_pandas#3895
SkxOverKill wants to merge 2 commits into
google:masterfrom
SkxOverKill:fix/to_pandas-timeline-id-field

Conversation

@SkxOverKill

@SkxOverKill SkxOverKill commented Aug 11, 2026

Copy link
Copy Markdown

Summary

Fixes timeline-name mapping in Search.to_pandas() when _source or __ts_timeline_id is requested as a return field.

Root Cause

The field-mapping block contained two related defects:

  1. The __ts_timeline_id branch assigned its result to _source, so callers received a spurious or overwritten _source value instead of the requested __ts_timeline_id column.
  2. Both branches read __ts_timeline_id from the top level of the OpenSearch hit. The ID is stored within the event's _source, so the lookup returned None instead of the timeline name.

Timeline IDs may also be represented as strings in event data while sketch timeline objects expose integer IDs. Without normalization, valid string IDs silently miss the timeline lookup.

Implementation

  • Read the timeline ID from the event source.
  • Normalize string-form IDs for lookup against the sketch timeline map.
  • Populate _source and __ts_timeline_id independently when requested.
  • Preserve the original timeline ID when no matching timeline exists, avoiding loss of event provenance.

Test Coverage

The regression test covers:

  • integer timeline IDs;
  • string timeline IDs;
  • unknown timeline IDs and fallback behavior;
  • both _source and __ts_timeline_id output columns.

Verification

python -m pytest api_client/python/timesketch_api_client/search_test.py -q
9 passed in 3.12s

python -m pylint --rcfile=.pylintrc \
  api_client/python/timesketch_api_client/search.py \
  api_client/python/timesketch_api_client/search_test.py
Passed

python -m black --check --diff \
  api_client/python/timesketch_api_client/search.py \
  api_client/python/timesketch_api_client/search_test.py
2 files would be left unchanged

@google-cla

google-cla Bot commented Aug 11, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@jaegeral

Copy link
Copy Markdown
Collaborator

/gemini review

@jaegeral jaegeral self-assigned this Aug 12, 2026
@jkppr jkppr assigned SkxOverKill and unassigned jaegeral Aug 12, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the to_pandas method in the Python API client to map the __ts_timeline_id field to its corresponding timeline name, and adds a unit test to verify this behavior. The review feedback highlights potential issues with timeline lookups, specifically a type mismatch if the timeline ID is a string and the lack of a fallback if the timeline is not found. It suggests casting the ID to an integer and falling back to the original ID if the lookup fails, along with updating the unit test to cover string IDs.

Comment thread api_client/python/timesketch_api_client/search.py Outdated
Comment thread api_client/python/timesketch_api_client/search_test.py Outdated
@SkxOverKill

Copy link
Copy Markdown
Author

Thanks for the review. I pushed b31705b addressing both findings: string-form timeline IDs are normalized for lookup, unknown IDs retain their original value as a fallback, and the regression now covers integer, string, and missing IDs across both affected output columns. I also fixed the Black and PyLint failures. The PR description now documents the behavior and verification results.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants