Re: Prometheus Reporter Enhancement

2021-06-03 Thread Chesnay Schepler

Let's move this to the ticket then. :)

On 6/2/2021 8:45 PM, Mason Chen wrote:

Hi Chesnay,

I would like to take on 
https://issues.apache.org/jira/browse/FLINK-17495 
 as a contribution 
to OSS, if that’s alright with the team. We can discuss implementation 
details here or in the ticket, but I was thinking about modifying the 
ReporterScopedSettings to enable this generic tag support.


Best,
Mason

On May 20, 2021, at 4:36 AM, Chesnay Schepler > wrote:


There is no plan to generally exclude label keys from the metric 
identifier/logical scope. They ensure that the label set for a given 
identifier/scope is unique, i.e., you can't have 2 metrics called 
"numRecordsIn" with different label sets. Changing this would also 
break all existing setups, so if anything if would have to be an 
opt-in feature.


What I envision more is for the user to have more control over the 
metric identifier/logical scope via the scope formats. They are 
currently quite limited by only controlling part of the final 
identifier, while the logical scope isn't controllable at all.


Generally though, there's a fair bit of internal re-structuring that 
we'd like to do before extending the metric system further, because 
we've been tacking on more and more things since it was released in 
1.3.0 (!!!) but barely refactored things to properly fit together.


On 5/20/2021 12:58 AM, Mason Chen wrote:
Are there any plans to rework some of the metric name formulations 
(getMetricIdentifier or getLogicalScope)? Currently, the label keys 
and/or label values are concatenated in the metric name and the 
information is redundant and makes the metric names longer.


Would it make sense to remove the tag related information 
(getAllVariables())?


On May 18, 2021, at 3:45 PM, Chesnay Schepler > wrote:


There is already a ticket for this. Note that this functionality 
should be implemented in a generic fashion to be usable for all 
reporters.


https://issues.apache.org/jira/browse/FLINK-17495

On 5/18/2021 8:16 PM, Andrew Otto wrote:

Sounds useful!

On Tue, May 18, 2021 at 2:02 PM Mason Chen > wrote:


Hi all,

Would people appreciate enhancements to the prometheus
reporter to include extra labels via a configuration, as a
contribution to Flink? I can see it being useful for adding
labels that are not job specific, but infra specific.

The change would be nicely integrated with the Flink’s
ConfigOptions and unit tested.

Best,
Mason













Re: Prometheus Reporter Enhancement

2021-06-02 Thread Mason Chen
Hi Chesnay,

I would like to take on https://issues.apache.org/jira/browse/FLINK-17495 
 as a contribution to OSS, 
if that’s alright with the team. We can discuss implementation details here or 
in the ticket, but I was thinking about modifying the ReporterScopedSettings to 
enable this generic tag support.

Best,
Mason

> On May 20, 2021, at 4:36 AM, Chesnay Schepler  wrote:
> 
> There is no plan to generally exclude label keys from the metric 
> identifier/logical scope. They ensure that the label set for a given 
> identifier/scope is unique, i.e., you can't have 2 metrics called 
> "numRecordsIn" with different label sets. Changing this would also break all 
> existing setups, so if anything if would have to be an opt-in feature.
> 
> What I envision more is for the user to have more control over the metric 
> identifier/logical scope via the scope formats. They are currently quite 
> limited by only   controlling part of the final identifier, while the 
> logical scope isn't controllable at all.
> 
> Generally though, there's a fair bit of internal re-structuring that we'd 
> like to do before extending the metric system further, because we've been 
> tacking on more and more things since it was released in 1.3.0 (!!!) but 
> barely refactored things to properly fit together.
> 
> On 5/20/2021 12:58 AM, Mason Chen wrote:
>> Are there any plans to rework some of the metric name formulations 
>> (getMetricIdentifier or getLogicalScope)? Currently, the label keys and/or 
>> label values are concatenated in the metric name and the information is 
>> redundant and makes the metric names longer.
>> 
>> Would it make sense to remove the tag related information 
>> (getAllVariables())?
>> 
>>> On May 18, 2021, at 3:45 PM, Chesnay Schepler >> > wrote:
>>> 
>>> There is already a ticket for this. Note that this functionality should be 
>>> implemented in a generic fashion to be usable for all reporters.
>>> 
>>> https://issues.apache.org/jira/browse/FLINK-17495 
>>> 
>>> 
>>> On 5/18/2021 8:16 PM, Andrew Otto wrote:
 Sounds useful!
 
 On Tue, May 18, 2021 at 2:02 PM Mason Chen >>> > wrote:
 Hi all,
 
 Would people appreciate enhancements to the prometheus reporter to include 
 extra labels via a configuration, as a contribution to Flink? I can see it 
 being useful for adding labels that are not job specific, but infra 
 specific.
 
 The change would be nicely integrated with the Flink’s ConfigOptions and 
 unit tested.
 
 Best,
 Mason
>>> 
>> 
> 



Re: Prometheus Reporter Enhancement

2021-05-20 Thread Chesnay Schepler
There is no plan to generally exclude label keys from the metric 
identifier/logical scope. They ensure that the label set for a given 
identifier/scope is unique, i.e., you can't have 2 metrics called 
"numRecordsIn" with different label sets. Changing this would also break 
all existing setups, so if anything if would have to be an opt-in feature.


What I envision more is for the user to have more control over the 
metric identifier/logical scope via the scope formats. They are 
currently quite limited by only controlling part of the final 
identifier, while the logical scope isn't controllable at all.


Generally though, there's a fair bit of internal re-structuring that 
we'd like to do before extending the metric system further, because 
we've been tacking on more and more things since it was released in 
1.3.0 (!!!) but barely refactored things to properly fit together.


On 5/20/2021 12:58 AM, Mason Chen wrote:
Are there any plans to rework some of the metric name formulations 
(getMetricIdentifier or getLogicalScope)? Currently, the label keys 
and/or label values are concatenated in the metric name and the 
information is redundant and makes the metric names longer.


Would it make sense to remove the tag related information 
(getAllVariables())?


On May 18, 2021, at 3:45 PM, Chesnay Schepler > wrote:


There is already a ticket for this. Note that this functionality 
should be implemented in a generic fashion to be usable for all 
reporters.


https://issues.apache.org/jira/browse/FLINK-17495

On 5/18/2021 8:16 PM, Andrew Otto wrote:

Sounds useful!

On Tue, May 18, 2021 at 2:02 PM Mason Chen > wrote:


Hi all,

Would people appreciate enhancements to the prometheus reporter
to include extra labels via a configuration, as a contribution
to Flink? I can see it being useful for adding labels that are
not job specific, but infra specific.

The change would be nicely integrated with the Flink’s
ConfigOptions and unit tested.

Best,
Mason









Re: Prometheus Reporter Enhancement

2021-05-19 Thread Mason Chen
Are there any plans to rework some of the metric name formulations 
(getMetricIdentifier or getLogicalScope)? Currently, the label keys and/or 
label values are concatenated in the metric name and the information is 
redundant and makes the metric names longer.

Would it make sense to remove the tag related information (getAllVariables())?

> On May 18, 2021, at 3:45 PM, Chesnay Schepler  wrote:
> 
> There is already a ticket for this. Note that this functionality should be 
> implemented in a generic fashion to be usable for all reporters.
> 
> https://issues.apache.org/jira/browse/FLINK-17495 
> 
> 
> On 5/18/2021 8:16 PM, Andrew Otto wrote:
>> Sounds useful!
>> 
>> On Tue, May 18, 2021 at 2:02 PM Mason Chen > > wrote:
>> Hi all,
>> 
>> Would people appreciate enhancements to the prometheus reporter to include 
>> extra labels via a configuration, as a contribution to Flink? I can see it 
>> being useful for adding labels that are not job specific, but infra specific.
>> 
>> The change would be nicely integrated with the Flink’s ConfigOptions and 
>> unit tested.
>> 
>> Best,
>> Mason
> 



Re: Prometheus Reporter Enhancement

2021-05-18 Thread Chesnay Schepler
There is already a ticket for this. Note that this functionality should 
be implemented in a generic fashion to be usable for all reporters.


https://issues.apache.org/jira/browse/FLINK-17495

On 5/18/2021 8:16 PM, Andrew Otto wrote:

Sounds useful!

On Tue, May 18, 2021 at 2:02 PM Mason Chen > wrote:


Hi all,

Would people appreciate enhancements to the prometheus reporter to
include extra labels via a configuration, as a contribution to
Flink? I can see it being useful for adding labels that are not
job specific, but infra specific.

The change would be nicely integrated with the Flink’s
ConfigOptions and unit tested.

Best,
Mason





Re: Prometheus Reporter Enhancement

2021-05-18 Thread Andrew Otto
Sounds useful!

On Tue, May 18, 2021 at 2:02 PM Mason Chen  wrote:

> Hi all,
>
> Would people appreciate enhancements to the prometheus reporter to include
> extra labels via a configuration, as a contribution to Flink? I can see it
> being useful for adding labels that are not job specific, but infra
> specific.
>
> The change would be nicely integrated with the Flink’s ConfigOptions and
> unit tested.
>
> Best,
> Mason
>


Prometheus Reporter Enhancement

2021-05-18 Thread Mason Chen
Hi all,

Would people appreciate enhancements to the prometheus reporter to include 
extra labels via a configuration, as a contribution to Flink? I can see it 
being useful for adding labels that are not job specific, but infra specific.

The change would be nicely integrated with the Flink’s ConfigOptions and unit 
tested.

Best,
Mason