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

Add the ability to proxy metrics via http proxy #574

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

achulkov2
Copy link
Collaborator

Implements #549. HTTP proxies now have an additional /solomon_proxy/sensors handle, which can be used to collect metrics from internal YT components without network access to their monitoring ports.

There are quite a few TODO-s left, but they are immediately needed and can be isolated to separate pull requests.

Additional endpoint providers (for CHYT cliques, for example) will be implemented later.

http_proxy_config = configs["http_proxy"][0]
scheduler_config = configs["scheduler"][0]


Copy link
Member

Choose a reason for hiding this comment

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

nip: extra blanck line


@authors("achulkov2")
def test_formats(self):
# Json (default)
Copy link
Member

Choose a reason for hiding this comment

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

nip: dot


# Solomon shards (which are also instance labels).
assert self.get_instance_count(self.get_sensors(params={"instance_shard": "all"})) == 3
assert self.get_instance_count(self.get_sensors(params={"instance_shard": "we-miss-prime"})) == 0
Copy link
Member

Choose a reason for hiding this comment

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

So true!


////////////////////////////////////////////////////////////////////////////////

TComponentDiscoverer::TComponentDiscoverer(IClientPtr client, TMasterReadOptions masterReadOptions, TDuration proxyDeathAge)
Copy link
Member

Choose a reason for hiding this comment

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

I'd suggest split arguments by lines

.ValueOrThrow();
auto rspList = ConvertToNode(rsp)->AsList();

std::vector<TYtInstance> instances;
Copy link
Member

Choose a reason for hiding this comment

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

nip: reserve

return pullHeaders;
}

TCgiParameters TSolomonProxy::PreparePullParameters(const TCgiParameters& params)
Copy link
Member

Choose a reason for hiding this comment

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

nip: params -> parameters globally?

return filteredEndpoints;
}

std::vector<TString> TSolomonProxy::CollectEndpoints(const TCgiParameters& params, int shard_index, int shard_count) const
Copy link
Member

Choose a reason for hiding this comment

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

shardIndex, shardCount

auto reqUrlRef = req->GetUrl();
TCgiParameters params(reqUrlRef.RawQuery);

auto shard_index = ParseIntegerParameter(params, ShardIndexParameterName, /*defaultValue*/ 0);
Copy link
Member

Choose a reason for hiding this comment

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

shardIndex, shardCount


// TODO(achulkov2): Ideally, we would want some sort of memory accounting here.
if (std::ssize(filteredEndpoints) > Config_->MaxEndpointsPerRequest) {
THROW_ERROR_EXCEPTION("Cannot pull sensors from %v endpoints at once, please retry the request with a larger shard count", filteredEndpoints.size())
Copy link
Member

Choose a reason for hiding this comment

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

please :)

<< TErrorAttribute("max_endpoints_per_request", Config_->MaxEndpointsPerRequest);
}

std::vector<TFuture<IResponsePtr>> asyncPullResponses;
Copy link
Member

Choose a reason for hiding this comment

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

reserve?

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

2 participants