Skip to content

Add redis to filibuster opentelemetry library - #33

Open
eunicech wants to merge 10 commits into
mainfrom
add-redis
Open

Add redis to filibuster opentelemetry library#33
eunicech wants to merge 10 commits into
mainfrom
add-redis

Conversation

@eunicech

Copy link
Copy Markdown
Collaborator

No description provided.

@cmeiklejohn cmeiklejohn 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.

What is this doing precisely and why did you have to make this change?

@cmeiklejohn cmeiklejohn self-assigned this Mar 30, 2022
@cmeiklejohn

Copy link
Copy Markdown
Contributor

Please fix commit f2a8a9d to comment out, rather than remove the code and leave a comment as to the error that's thrown when that code is enabled. This will allow us to precisely identify what we removed and why.

Comment thread filibuster/instrumentation/redis/__init__.py Outdated
Comment thread filibuster/instrumentation/redis/__init__.py Outdated
# we should be able to avoid this -- it's because we're returning an invalid
# object through the opentelemetry instrumentation.
#
result = Response()

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’m still confused here. You shouldn’t be picking the type you want to return here. This should be the type of response that is returned when Redis naturally returns an error. Therefore, whether or not Redis actually experiences an error or we use fault injection to create one it returns the same type.

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.

For example, when you are issuing HTTP requests with requests, three different things can happen:

  1. First, the call might succeed, and it returns to the caller an instance of Response with the fields set appropriately.
  2. Second, the call might fail because of timeout or connection error. In this case, it doesn’t return anything, but instead throws an exception in the requests.exceptions namespace.`
  3. Third, the call might return a response indicating error. For example, the remote service might return HTTP 404 NotFound or HTTP 500 InternalServerError. In this case, a Response object is returned with the appropriate status code and body set.

Therefore, the first thing is to figure out what the type of object being returned back to the caller, using the Redis library is. That’s the type you’ll want to use. When injecting faults that are the non-throwable kind, you’ll also want to ensure you instantiate that type and make it look — when injecting a fault that we create — indistinguishable from an actual fault.

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