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

VAULT-14733: SegmentReader interface for reading activity log segments #19934

Merged
merged 3 commits into from
Apr 6, 2023

Conversation

miagilepner
Copy link
Contributor

@miagilepner miagilepner commented Apr 3, 2023

This PR adds a SegmentReader interface and an implementation to read segment files from storage for use in the activity log.

This makes it easier to allow new inputs (such as mock data) to the precomputed query worker.

@miagilepner miagilepner marked this pull request as ready for review April 3, 2023 11:58
@miagilepner miagilepner added this to the 1.14 milestone Apr 3, 2023
vault/activity_log_util_common.go Outdated Show resolved Hide resolved
vault/activity_log_util_common_test.go Outdated Show resolved Hide resolved
@miagilepner miagilepner requested review from mpalmi and a team April 4, 2023 12:47
Copy link
Contributor

@hghaf099 hghaf099 left a comment

Choose a reason for hiding this comment

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

Nice work! Some minor comments, otherwise looks very good!


// writeSegment writes a single segment file with the given time and index
// the correct path is inferred from the datatype of `item`
func writeSegment(t *testing.T, core *Core, ts time.Time, index int, item proto.Message) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would you think instead of having a generic function in which we would need to add logic to distinguish which item type we are dealing with, having two functions writeTokenSegment and writeEntitySegment would be more readable? This way, we could also remove the makeSegmentPath function as well, as that could be reduced to just a fmt.Sprintf call.

vault/activity_log_util_common.go Show resolved Hide resolved
vault/activity_log_util_common.go Outdated Show resolved Hide resolved
vault/activity_log_util_common.go Show resolved Hide resolved
Copy link
Contributor

@hghaf099 hghaf099 left a comment

Choose a reason for hiding this comment

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

Looks good!

@miagilepner miagilepner merged commit d70c17f into main Apr 6, 2023
miagilepner added a commit that referenced this pull request Apr 6, 2023
#19934)

* create a segment reader for activity log segment

* fix imports

* updates based on comments
miagilepner added a commit that referenced this pull request Apr 6, 2023
#19934)

* create a segment reader for activity log segment

* fix imports

* updates based on comments
miagilepner added a commit that referenced this pull request Apr 11, 2023
… log segments into release/1.12.x (#20017)

* VAULT-14733: SegmentReader interface for reading activity log segments (#19934)

* create a segment reader for activity log segment

* fix imports

* updates based on comments

* format

---------

Co-authored-by: miagilepner <mia.epner@hashicorp.com>
miagilepner added a commit that referenced this pull request Apr 11, 2023
… log segments into release/1.11.x (#20016)

* VAULT-14733: SegmentReader interface for reading activity log segments (#19934)

* create a segment reader for activity log segment

* fix imports

* updates based on comments

* format

* add missing import

---------

Co-authored-by: miagilepner <mia.epner@hashicorp.com>
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.

3 participants