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

HLRC: Add xpack usage api #31975

Merged
merged 5 commits into from
Jul 13, 2018
Merged

HLRC: Add xpack usage api #31975

merged 5 commits into from
Jul 13, 2018

Conversation

rjernst
Copy link
Member

@rjernst rjernst commented Jul 11, 2018

This commit adds the _xpack/usage api to the high level rest client.
Currently in the transport api, the usage data is exposed in a limited
fashion, at most giving one level of helper methods for the inner keys
of data, but then exposing thos subobjects as maps of objects. Rather
than making parsers for every set of usage data from each feature, this
PR exposes the entire set of usage data as a map of maps.

This commit adds the _xpack/usage api to the high level rest client.
Currently in the transport api, the usage data is exposed in a limited
fashion, at most giving one level of helper methods for the inner keys
of data, but then exposing thos subobjects as maps of objects. Rather
than making parsers for every set of usage data from each feature, this
PR exposes the entire set of usage data as a map of maps.
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

@rjernst rjernst requested a review from nik9000 July 11, 2018 19:31
Map<String, Object> monitoringUsage = usages.get("monitoring");
assertThat(monitoringUsage.get("available"), is(true));
assertThat(monitoringUsage.get("enabled"), is(true));
assertThat(monitoringUsage.get("collection_enabled"), is(false));
Copy link
Member

Choose a reason for hiding this comment

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

Missing and //end: tag.

==== Execution

Detailed information about the usage of features from {xpack} can be
retrieved using the `xpackUsage()` method:
Copy link
Member

Choose a reason for hiding this comment

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

The name isn't quite right here. I suspect the name in xpackInfo isn't right either now that I see this. I bet I didn't catch that in the rename....

Copy link
Member Author

Choose a reason for hiding this comment

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

I think the name is correct. What do you think it should be?

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah ic, xpackUsage is the helper method in RequestConverters.

* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License;
* you may not use this file except in compliance with the Elastic License.
* Licensed to Elasticsearch under one or more contributor
Copy link
Member

Choose a reason for hiding this comment

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

I think this'll cause the build to fail.

Copy link
Member Author

Choose a reason for hiding this comment

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

Silly IntelliJ :/

import org.elasticsearch.action.ActionRequestValidationException;

/** A dummy request object solely for the purpose of boilerplate. */
public class XPackUsageRequest extends ActionRequest {
Copy link
Member

Choose a reason for hiding this comment

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

It looks to me like there is a master_node_timeout that you can set on the request. Maybe we move the one in x-pack into the protocol and use it instead?

import java.util.Map;
import java.util.stream.Collectors;

public class XPackUsageResponse {
Copy link
Member

Choose a reason for hiding this comment

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

May as well stick javadoc on it. It'll be mostly ceremonial but it might help someone who hasn't figured out the naming convention yet.

@rjernst
Copy link
Member Author

rjernst commented Jul 12, 2018

@nik9000 I think I have addressed all your comments.

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

I left two tiny things. LGTM otherwise.

@@ -1096,6 +1097,11 @@ static Request xPackInfo(XPackInfoRequest infoRequest) {
return request;
}

@SuppressWarnings("unused")
Copy link
Member

Choose a reason for hiding this comment

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

I think this is used.

@@ -1096,6 +1097,11 @@ static Request xPackInfo(XPackInfoRequest infoRequest) {
return request;
}

@SuppressWarnings("unused")
static Request xpackUsage(XPackUsageRequest request) {
return new Request(HttpGet.METHOD_NAME, "/_xpack/usage");
Copy link
Member

Choose a reason for hiding this comment

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

Can you add the master timeout?

Copy link
Member

@nik9000 nik9000 left a comment

Choose a reason for hiding this comment

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

❤️

@rjernst rjernst merged commit 2c3ea43 into elastic:master Jul 13, 2018
@rjernst rjernst deleted the hlrc_xpack_usage branch July 13, 2018 16:33
rjernst added a commit that referenced this pull request Jul 14, 2018
This commit adds the _xpack/usage api to the high level rest client.
Currently in the transport api, the usage data is exposed in a limited
fashion, at most giving one level of helper methods for the inner keys
of data, but then exposing thos subobjects as maps of objects. Rather
than making parsers for every set of usage data from each feature, this
PR exposes the entire set of usage data as a map of maps.
martijnvg added a commit that referenced this pull request Jul 16, 2018
* es/6.x:
  Use correct formatting for links (#29460)
  Revert "Adds a new auto-interval date histogram (#28993)"
  Revert "fix typo"
  fix typo
  Adds a new auto-interval date histogram (#28993)
  [Rollup] Replace RollupIT with a ESRestTestCase version (#31977)
  [Rollup] Fix duplicate field names in test (#32075)
  [Tests] Fix failure due to changes exception message (#32036)
  [Test] Mute MlJobIT#testDeleteJobAfterMissingAliases
  Replace Ingest ScriptContext with Custom Interface (#32003) (#32060)
  Cleanup Duplication in `PainlessScriptEngine` (#31991) (#32061)
  HLRC: Add xpack usage api (#31975)
  Clean Up Snapshot Create Rest API (#31779)
@jimczi jimczi added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants