mmiklavc commented on a change in pull request #1458: METRON-2177 Upgrade
Profiler for HBase 2.0.2
URL: https://github.com/apache/metron/pull/1458#discussion_r311132873
##########
File path:
metron-analytics/metron-profiler-client/src/main/java/org/apache/metron/profiler/client/stellar/GetProfile.java
##########
@@ -102,94 +90,108 @@
returns="The selected profile measurements."
)
public class GetProfile implements StellarFunction {
+ private static final Logger LOG =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
/**
- * Cached client that can retrieve profile values.
+ * Allows the function to retrieve persisted {@link ProfileMeasurement}
values.
*/
- private ProfilerClient client;
+ private ProfilerClient profilerClient;
/**
- * Cached value of config map actually used to construct the previously
cached client.
+ * Creates the {@link ProfilerClient} used by this function.
*/
- private Map<String, Object> cachedConfigMap = new HashMap<String, Object>(6);
-
- private static final Logger LOG =
LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
+ private ProfilerClientFactory profilerClientFactory;
/**
- * Initialization. No longer need to do anything in initialization,
- * as all setup is done lazily and cached.
+ * Last known global configuration used to create the {@link
ProfilerClient}. If the
+ * global configuration changes, a new {@link ProfilerClient} needs to be
constructed.
*/
+ private Map<String, Object> lastKnownGlobals = new HashMap<>();
+
+ public GetProfile() {
+ profilerClientFactory = new HBaseProfilerClientFactory();
+ }
+
@Override
public void initialize(Context context) {
+ Map<String, Object> globals = getGlobals(context);
+ profilerClient = profilerClientFactory.create(globals);
}
- /**
- * Is the function initialized?
- */
@Override
public boolean isInitialized() {
- return true;
+ return profilerClient != null;
}
- /**
- * Apply the function.
- * @param args The function arguments.
- * @param context
- */
@Override
- public Object apply(List<Object> args, Context context) throws
ParseException {
+ public void close() throws IOException {
+ if(profilerClient != null) {
+ profilerClient.close();
+ }
+ }
+ @Override
+ public Object apply(List<Object> args, Context context) throws
ParseException {
+ // required arguments
String profile = getArg(0, String.class, args);
String entity = getArg(1, String.class, args);
- Optional<List<ProfilePeriod>> periods = Optional.ofNullable(getArg(2,
List.class, args));
- //Optional arguments
- @SuppressWarnings("unchecked")
- List<Object> groups = null;
- Map configOverridesMap = null;
- if (args.size() < 4) {
- // no optional args, so default 'groups' and configOverridesMap remains
null.
- groups = new ArrayList<>(0);
- }
- else if (args.get(3) instanceof List) {
- // correct extensible usage
- groups = getArg(3, List.class, args);
- if (args.size() >= 5) {
- configOverridesMap = getArg(4, Map.class, args);
- if (configOverridesMap.isEmpty()) configOverridesMap = null;
- }
- }
- else {
- // Deprecated "varargs" style usage for groups_list
- // configOverridesMap cannot be specified so it remains null.
- groups = getGroupsArg(3, args);
- }
+ List<ProfilePeriod> periods = getArg(2, List.class, args);
- Map<String, Object> effectiveConfig = getEffectiveConfig(context,
configOverridesMap);
- Object defaultValue = null;
- //lazily create new profiler client if needed
- if (client == null || !cachedConfigMap.equals(effectiveConfig)) {
- RowKeyBuilder rowKeyBuilder = getRowKeyBuilder(effectiveConfig);
- ColumnBuilder columnBuilder = getColumnBuilder(effectiveConfig);
- HTableInterface table = getTable(effectiveConfig);
- long periodDuration = getPeriodDurationInMillis(effectiveConfig);
- client = new HBaseProfilerClient(table, rowKeyBuilder, columnBuilder,
periodDuration);
- cachedConfigMap = effectiveConfig;
- }
- if(cachedConfigMap != null) {
- defaultValue =
ProfilerClientConfig.PROFILER_DEFAULT_VALUE.get(cachedConfigMap);
+ // optional arguments
+ List<Object> groups = getGroups(args);
+ Map<String, Object> overrides = getOverrides(args);
+
+ // lazily create new profiler client if needed
+ Map<String, Object> effectiveConfig = getEffectiveConfig(context,
overrides);
+ if (profilerClient == null || !lastKnownGlobals.equals(effectiveConfig)) {
+ profilerClient = profilerClientFactory.create(effectiveConfig);
+ lastKnownGlobals = effectiveConfig;
}
- List<ProfileMeasurement> measurements = client.fetch(Object.class,
profile, entity, groups,
- periods.orElse(new ArrayList<>(0)),
Optional.ofNullable(defaultValue));
+ // is there a default value?
+ Optional<Object> defaultValue = Optional.empty();
+ if(effectiveConfig != null) {
+ defaultValue =
Optional.ofNullable(PROFILER_DEFAULT_VALUE.get(effectiveConfig));
+ }
// return only the value of each profile measurement
+ List<ProfileMeasurement> measurements = profilerClient.fetch(Object.class,
profile, entity, groups, periods, defaultValue);
List<Object> values = new ArrayList<>();
for(ProfileMeasurement m: measurements) {
values.add(m.getProfileValue());
}
+
return values;
}
+ private Map<String, Object> getOverrides(List<Object> args) {
Review comment:
I hear you - I think this is one instance where it's definitely worth the
burden to improve the readability since you've already touched those lines of
code refactoring anyways. Naming constants is a pretty common standard
practice, versus other refactoring that could be considered stylistic/personal
choices on readability, etc.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
