Client-side LB code region reporting change request acknowledgment

[maesta2]

Is it possible to get this changed so client side LB code reports the actual region: https://github.com/scylladb/alternator-load-balancing/blob/master/go/v2/alternator_lb.go#L53?

[mykaul]

Is it possible to get this changed so client side LB code reports the actual region: https://github.com/scylladb/alternator-load-balancing/blob/master/go/v2/alternator_lb.go#L53?

Reports to whom? @maesta2 - what's the bug that you are concerned with?

[maesta2]

@mykaul if you look at the source of above link, it seems that below function does not return Region info and that is what the user was asking.

func (n *AlternatorNodes) Config(fake_domain string, key string, secret_key string) aws.Config {
	fake_url := fmt.Sprintf("%s://%s:%d", n.scheme, fake_domain, n.port)

	return aws.Config{
		EndpointResolverWithOptions: staticEndpointResolver(fake_url),
		// Region is used in the signature algorithm so prevent request sent
		// to one region to be forward by an attacker to a different region.
		// But Alternator doesn't check it. It can be anything.
		Region: "whatever",
		// The third credential below, the session token, is only used for
		// temporary credentials, and is not supported by Alternator anyway.
		Credentials: credentials.NewStaticCredentialsProvider(key, secret_key, ""),

		APIOptions: []func(*middleware.Stack) error{
			func(m *middleware.Stack) error {
				return m.Finalize.Add(n.loadBalancerMiddleware(fake_domain), middleware.Before)
			},
		},
	}
}

[mykaul]

@mykaul if you look at the source of above link, it seems that below function does not return Region info and that is what the user was asking.

Yes, I've looked at the source code and the comment there. If that's an enterprise customer, please open a separate enterprise issue. It's not clear to me what the request is. The data center name? As you can see, there's not yet a check around it in Alternator.
See also scylladb/scylladb#12147

[dkropachev]

@maesta2 , couple cents to add to @mykaul, alternator is runnign on top of scylla, in scylla there is no Region, only Datacenter and Rack.
And technically both can represent Cloud Region, you can have nodes of one Rack be running on one AWS/GCP Region, while nodes from other Rack on another, and you can do the same for Datacenter.
Beside that there are many other possible configurations, so it is up to deployment and configuration what represents what.
On Scylla Cloud, for example, Datacenter represents Cloud Region, even though naming is bit different.

In the code you have provided client creates aws.Config which is used by dynamodb.NewFromConfig to forge requests to Alternator, it does not go anywhere else.
Alternator ignores this field, that is why you see random hardcoded string there, now question is, why do you want to change it?

[maesta2]

@dkropachev the answer to your question:

basically the problem we are having is we have instrumented our AWS SDK with open telemetry (https://pkg.go.dev/go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws), and since swapping to use this client side load balancing lib we get the region "whatever" propagated through our traces where we used to get an AWS region name

[dkropachev]

@dkropachev the answer to your question:

basically the problem we are having is we have instrumented our AWS SDK with open telemetry (https://pkg.go.dev/go.opentelemetry.io/contrib/instrumentation/github.com/aws/aws-sdk-go-v2/otelaws), and since swapping to use this client side load balancing lib we get the region "whatever" propagated through our traces where we used to get an AWS region name

Now it makes sense, we can make it configurable to allow you to see whatever you want there, is it good enough?

[maesta2]

@dkropachev Yes, it would be enough.

[dkropachev]

@dkropachev Yes, it would be enough.

Then following should be enough:

	cfg := alternatorNodes.Config("dog.scylladb.com", "alternator", "secret_pass")
        cfg.Region = "eu-west-1"

[maesta2]

@dkropachev when will it be merged?

[dkropachev]

@dkropachev when will it be merged?

What I meant is that you can do that in your app to see the region.

[maesta2]

@dkropachev Would it be nice to port this to the code so that I don't have ask the customer to write their own code?

[dkropachev]

@dkropachev Would it be nice to port this to the code so that I don't have ask the customer to write their own code?

I am pretty sure there is no other way to use code from this repo but write your own code.

[maesta2]

Thanks. The customer agreed to change it by themselves.