cpoerschke commented on code in PR #2382: URL: https://github.com/apache/solr/pull/2382#discussion_r1603697031
########## solr/modules/monitor/src/java/org/apache/lucene/monitor/MonitorFields.java: ########## @@ -0,0 +1,38 @@ +/* + * + * * Licensed to the Apache Software Foundation (ASF) under one or more + * * contributor license agreements. See the NOTICE file distributed with + * * this work for additional information regarding copyright ownership. + * * The ASF licenses this file to You under the Apache License, Version 2.0 + * * (the "License"); you may not use this file except in compliance with + * * the License. You may obtain a copy of the License at + * * + * * http://www.apache.org/licenses/LICENSE-2.0 + * * + * * Unless required by applicable law or agreed to in writing, software + * * distributed under the License is distributed on an "AS IS" BASIS, + * * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * * See the License for the specific language governing permissions and + * * limitations under the License. + * + */ + +package org.apache.lucene.monitor; + +import java.util.Set; + +public class MonitorFields { + + public static final String QUERY_ID = QueryIndex.FIELDS.query_id + "_"; + public static final String CACHE_ID = QueryIndex.FIELDS.cache_id + "_"; + public static final String MONITOR_QUERY = QueryIndex.FIELDS.mq + "_"; + public static final String PAYLOAD = QueryIndex.FIELDS.mq + "_payload_"; Review Comment: > ... is there a particular reason you were exploring making the payload field name overridable? Maybe to let users give it a more self-describing name for their particular use case? ... So I was exploring not specifically w.r.t. making the payload field name overridable but the other fields (query id, cache id, query) also and payload was just first to explore. Yes, optionally-choosable names would let users choose names/terminology to suit their use case, and perhaps might it also then reduce or remove the need for the aliasing functionality (which I haven't yet looked into further)? As an aside, I noticed that the query id and cache id and query field name choices picked the Lucene `QueryIndex` fields -- https://github.com/apache/lucene/blob/releases/lucene/9.10.0/lucene/monitor/src/java/org/apache/lucene/monitor/QueryIndex.java#L46-L48 -- and suffixed an `_` underscore, is the suffixing needed or does it just make maybe debugging or so easier? -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
