Skip to content

Refactor context creation#433

Merged
shreyamalpani merged 4 commits intojordan.gonzalez/bottlecap/universal-instrumentationfrom
shreya.malpani/refactor-context-creation
Nov 5, 2024
Merged

Refactor context creation#433
shreyamalpani merged 4 commits intojordan.gonzalez/bottlecap/universal-instrumentationfrom
shreya.malpani/refactor-context-creation

Conversation

@shreyamalpani
Copy link
Copy Markdown
Contributor

What does this PR do?

Creates the context for a request_id on receiving the Invoke event, and removes this logic from other events since the Invoke event is always received first.

@shreyamalpani shreyamalpani requested a review from a team as a code owner November 4, 2024 15:26
/// `Context` is created and added to the buffer.
/// Creates a new `Context` and adds it to the buffer.
///
#[allow(clippy::ptr_arg)]
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.

Why do we need this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I could change it to &str instead of &String, but I wasn't sure if it is better to keep it consistent with the other functions using &String

pub fn create_context(&mut self, request_id: &str) {
        self.insert(Context::new(request_id.to_owned(), 0.0, 0.0, 0, None));
 }

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.

I just don't think it's worth passing a reference if we are gonna clone inside

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.

It doesn't have to be consistent because in the other ones you're searching, and in this one you're keeping the value.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I see, thanks!

@shreyamalpani shreyamalpani merged commit 376318f into jordan.gonzalez/bottlecap/universal-instrumentation Nov 5, 2024
@shreyamalpani shreyamalpani deleted the shreya.malpani/refactor-context-creation branch November 5, 2024 18:19
duncanista added a commit that referenced this pull request Nov 15, 2024
* create context on invoke event

* update tests

* clippy fixes

* remove `allow(clippy::ptr_arg)`

---------

Co-authored-by: jordan gonzález <30836115+duncanista@users.noreply.github.com>
duncanista added a commit that referenced this pull request Nov 19, 2024
* create context on invoke event

* update tests

* clippy fixes

* remove `allow(clippy::ptr_arg)`

---------

Co-authored-by: jordan gonzález <30836115+duncanista@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants