NIFI-15477 Add recordGauge method to ProcessSession #49
Conversation
| /** | ||
| * Record measurement value for the named Gauge, registering the named Gauge when not present in the system. |
There was a problem hiding this comment.
Could be useful addition in the javadoc for developers not familiar with the terminology
| /** | |
| * Record measurement value for the named Gauge, registering the named Gauge when not present in the system. | |
| /** | |
| * Record measurement value for the named Gauge, registering the named Gauge when not present in the system. | |
| * Unlike counters which track cumulative values, gauges represent point-in-time measurements that can increase or decrease. |
There was a problem hiding this comment.
Thanks, that is a helpful clarification to distinguish it from counters.
| * @param value Measurement value to record | ||
| * @param commitTiming Timing for when the measurement value should be committed | ||
| */ | ||
| default void recordGauge(String name, double value, CommitTiming commitTiming) { |
There was a problem hiding this comment.
Would it make sense to also have a convenience method like
default void recordGauge(String name, double value) {
recordGauge(name, value, CommitTiming.SESSION_COMMITTED);
}There was a problem hiding this comment.
I considered that possibility, but I could also see an argument for making NOW the default option. In the end, I think it is best to follow the pattern of adjustCounter, and require the argument in all cases.
| * See the License for the specific language governing permissions and | ||
| * limitations under the License. | ||
| */ | ||
| package org.apache.nifi.processor; |
There was a problem hiding this comment.
Is it the best package location in case it should apply to other components in the future? Maybe org.apache.nifi.metrics?
There was a problem hiding this comment.
It seems like a better fit under the processor package, since it is used directly in ProcessSession, and I would not expect this to be used outside the scope of ProcessSession. The same package also has DataUnit as an enum. I considered processor.metrics as a new package, but it would be the only member. I'll move it to processor.metrics.
|
Thanks again for the review @pvillard31! I marked this as ready for review following the passing vote thread for NIP-19 |
2 participants
Summary
NIFI-15477 Adds the
recordGaugemethod to theProcessSessioninterface as described in NIP-19 .The default implementation is empty, which will allow the NiFi API to be released and upgraded without having to make simultaneous changes to implement the new method.
The method signature follows the pattern of
adjustCounter, but uses theCommitTimingenumeration for clarity of reading and writing calls in Processors, as opposed to abooleanflag.Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000NIFI-00000Pull Request Formatting
mainbranch