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

Adding support for multiple DCs #416

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

ryakh
Copy link

@ryakh ryakh commented Sep 16, 2019

Reviving #262

Adding support for multiple DC's with a DCAwareRoundRobin load-balancing policy.

@@ -86,6 +86,28 @@
end
end

describe "#configure" do
Copy link
Contributor

@wflanagan wflanagan Oct 30, 2019

Choose a reason for hiding this comment

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

@ryakh This looks fine to me. I'm wondering if you can create a small test that pushes some data back and forth in both of these cases, so we all can be sure it works?

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1

def extract_cassandra_options(configuration)
configuration[:cassandra_options]
end

def extract_datacenter(configuration)
configuration.fetch(:datacenter, nil)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd prefer just configuration[:datacenter]

pezra and others added 8 commits January 2, 2021 10:41
Cequel is trying to work around the potential that ActiveSupport's
Module.delegate monkey patch exists. It reset's Forwardable's delegate
method to use ActiveSupport's delegate method definition.

But with Ruby 3 no longer handling mixed positional and keyword args,
this monkey patch no longer works. We've updated to handle the kwargs.
AM-493: Monkey patch Util::Forwardable to work with Ruby 3 kwargs
It makes it *highly* non-obvious how instrumentation is actually
happening / being set up.

Instead move it into a
`Cequel::Metal::DatadogInstrumentation.instrument!` method, which we can
call in the midst of setting up all other datadog instrumentation.
As we attempt to migrate to Datadog, we need Cassandra instrumentation to match what we currently use in New Relic.

Follow along with the built in New Relic instrumentation, but using Datadog's APIs.

Caveat:
The existing New Relic instrumentation attempts to patch the client code automatically based on if attempting to load the new relic library "works". We don't want to do this with Datadog. It makes the instrumentation "appear out of nowhere" (source: over an hour of me trying to figure out where New Relic instrumentation came from).

Instead, do the patching inside of a `Cequel::Metal::DatadogInstrumentation.instrument!` method, which we can call during an initializer along with all other Datadog instrumentation.
@stefannibrasil
Copy link

@awj @ryakh would it make sense to merge this branch to main? We are referencing to the latest commit SHA on the main convertkit app for a while.

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.

None yet

7 participants