Metrics & runtime attribute registration

classic Classic list List threaded Threaded
12 messages Options
Reply | Threaded
Open this post in threaded view
|

Metrics & runtime attribute registration

Tomaž Cerar-2
Hey guys,

We had interesting discussion with Brian on https://github.com/wildfly/wildfly-core/pull/848#issuecomment-119778986
about how we register runtime/metric attribute on resources.

There are many cases where subsystems only register attributes / resources only when server is booting into normal mode.
but if it server is booted only to "admin mode" all that runtime/metrics attributes are not registered and as such not seen in the model.

Registering runtime attributes only in normal mode can cause that results of :read-resource-description/read-resource
wouldn't return attributes that are on resources if server is started in admin mode or even CLI standalone.
Our metadata already provides information if attributes is runtime/metric/configuration.

This can cause problems for tooling that relies on output of those two operations.

Looking at current state of the code, we do use both ways of registering attributes either conditionally or always.
This probably originates from times where there was no good api/way to mark attribute/resource as runtime.

I am personally am in favor of always registering runtime attributes as this makes sure that user isn't surprised by some extra/missing
attributes based on fact how it is starting the server.


What do you guys think? Should we always register it or have it conditionally?

--
tomaz



_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Metrics & runtime attribute registration

Heiko Braun
+1 for ‘awalys'



On 13 Jul 2015, at 23:26, Tomaž Cerar <[hidden email]> wrote:

I am personally am in favor of always registering runtime attributes as this makes sure that user isn't surprised by some extra/missing
attributes based on fact how it is starting the server. 


_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Metrics & runtime attribute registration

Tristan Tarrant
Yes, always is best as it provides a more uniform environment for tests too.

Tristan

On 14/07/2015 09:00, Heiko Braun wrote:

> +1 for ‘awalys'
>
>
>
>> On 13 Jul 2015, at 23:26, Tomaž Cerar <[hidden email]
>> <mailto:[hidden email]>> wrote:
>>
>> I am personally am in favor of always registering runtime attributes
>> as this makes sure that user isn't surprised by some extra/missing
>> attributes based on fact how it is starting the server.
>
>
>
> _______________________________________________
> wildfly-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>

--
Tristan Tarrant
Infinispan Lead
JBoss, a division of Red Hat
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Metrics & runtime attribute registration

Brian Stansberry
I agree it's better to be uniform. There are a few things we need to
resolve though before making this the policy.

1) An attribute description includes the 'nillable' field, which
indicates whether a client can expect ModelType.UNDEFINED as the
attribute value. With this change, every metric may return
ModelType.UNDEFINED, since there cannot be a value if the process is in
--admin-only mode. I see two options:

a) For every metric we automatically record 'true' as the value for
'nillable'. The downside here is there is no way for a client to know if
ModelType.UNDEFINED is a valid value when the process isn't in
--admin-only.

b) We leave things as is, but clients can check if the 'access-type'
field has a value of 'metric' and use that to understand that the value
may sometimes be ModelType.UNDEFINED regardless of what 'nillable' says.

As I typed this I've formed a strong preference for a). To me b) is voodoo.


2) We reuse the same ResourceDefinition class on both a HostController
and on a server. The use on a HostController is to define the domain
level resource (e.g. /profile=*/subsystem=foo).

When a normal extension is running on a host controller, it *should not*
register metrics.[1] Just because we reuse a ResourceDefinition class
doesn't mean the resource is the same, and a /profile=*/subsystem=foo
resource is not the same as /subsystem=foo on a server. It's just a
chunk of config and does not have metrics.

The ExtensionContext has methods that can be used by extensions to
figure out this kind of state stuff. As part of implementing this policy
we'd change the behavior of the isRuntimeOnlyRegistrationValid() method
so hopefully this kind of change would be transparent to existing
extensions.

3) There a difference between registering a resource's definition and
actually handling a request for the metric. I expect most metric read
handlers are not written to function properly if the underlying services
are not there. Hopefully most will "just work" based on extending some
of our base handler classes, which will bypass execution in an
--admin-only server. But we should expect that this change will result
in some breakage that will have to be cleaned up as part of the work.

Cheers,
Brian


[1] Unless it's a host extension; which we can get into if anyone cares.

On 7/14/15 6:13 AM, Tristan Tarrant wrote:

> Yes, always is best as it provides a more uniform environment for tests too.
>
> Tristan
>
> On 14/07/2015 09:00, Heiko Braun wrote:
>> +1 for ‘awalys'
>>
>>
>>
>>> On 13 Jul 2015, at 23:26, Tomaž Cerar <[hidden email]
>>> <mailto:[hidden email]>> wrote:
>>>
>>> I am personally am in favor of always registering runtime attributes
>>> as this makes sure that user isn't surprised by some extra/missing
>>> attributes based on fact how it is starting the server.
>>
>>
>>
>> _______________________________________________
>> wildfly-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>
>


--
Brian Stansberry
Senior Principal Software Engineer
JBoss by Red Hat
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Metrics & runtime attribute registration

James Perkins
In reply to this post by Tomaž Cerar-2
If the attribute is going to be undefined what is the benefit of having it in the model? If the tooling is looking for an attribute that doesn't exist wouldn't they just check for the existence with ModelNode.has()?

I don't write any tooling so maybe I'm off :) I just don't see the benefit of adding an attribute that will always be UNDEFINED though.

On 07/13/2015 02:26 PM, Tomaž Cerar wrote:
Hey guys,

We had interesting discussion with Brian on https://github.com/wildfly/wildfly-core/pull/848#issuecomment-119778986
about how we register runtime/metric attribute on resources.

There are many cases where subsystems only register attributes / resources only when server is booting into normal mode.
but if it server is booted only to "admin mode" all that runtime/metrics attributes are not registered and as such not seen in the model.

Registering runtime attributes only in normal mode can cause that results of :read-resource-description/read-resource
wouldn't return attributes that are on resources if server is started in admin mode or even CLI standalone.
Our metadata already provides information if attributes is runtime/metric/configuration.

This can cause problems for tooling that relies on output of those two operations.

Looking at current state of the code, we do use both ways of registering attributes either conditionally or always.
This probably originates from times where there was no good api/way to mark attribute/resource as runtime.

I am personally am in favor of always registering runtime attributes as this makes sure that user isn't surprised by some extra/missing
attributes based on fact how it is starting the server.


What do you guys think? Should we always register it or have it conditionally?

--
tomaz




_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev

-- 
James R. Perkins
JBoss by Red Hat

_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Metrics & runtime attribute registration

Brian Stansberry
One thing that came up yesterday is generating code based on the model
description. If you're doing that you want as much data as possible. You
don't want to bring up an -admin-only process (say one embedded in the
code generation tool) just to read and generate, and then find out you
missed a bunch of attributes.

On 7/14/15 1:52 PM, James R. Perkins wrote:

> If the attribute is going to be undefined what is the benefit of having
> it in the model? If the tooling is looking for an attribute that doesn't
> exist wouldn't they just check for the existence with ModelNode.has()?
>
> I don't write any tooling so maybe I'm off :) I just don't see the
> benefit of adding an attribute that will always be UNDEFINED though.
>
> On 07/13/2015 02:26 PM, Tomaž Cerar wrote:
>> Hey guys,
>>
>> We had interesting discussion with Brian on
>> <https://github.com/wildfly/wildfly-core/pull/848#issuecomment-119778986>https://github.com/wildfly/wildfly-core/pull/848#issuecomment-119778986
>>
>> about how we register runtime/metric attribute on resources.
>>
>> There are many cases where subsystems only register attributes /
>> resources only when server is booting into normal mode.
>> but if it server is booted only to "admin mode" all that
>> runtime/metrics attributes are not registered and as such not seen in
>> the model.
>>
>> Registering runtime attributes only in normal mode can cause that
>> results of :read-resource-description/read-resource
>> wouldn't return attributes that are on resources if server is started
>> in admin mode or even CLI standalone.
>> Our metadata already provides information if attributes is
>> runtime/metric/configuration.
>>
>> This can cause problems for tooling that relies on output of those two
>> operations.
>>
>> Looking at current state of the code, we do use both ways of
>> registering attributes either conditionally or always.
>> This probably originates from times where there was no good api/way to
>> mark attribute/resource as runtime.
>>
>> I am personally am in favor of always registering runtime attributes
>> as this makes sure that user isn't surprised by some extra/missing
>> attributes based on fact how it is starting the server.
>>
>>
>> What do you guys think? Should we always register it or have it
>> conditionally?
>>
>> --
>> tomaz
>>
>>
>>
>>
>> _______________________________________________
>> wildfly-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>
> --
> James R. Perkins
> JBoss by Red Hat
>
>
>
> _______________________________________________
> wildfly-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>


--
Brian Stansberry
Senior Principal Software Engineer
JBoss by Red Hat
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Metrics & runtime attribute registration

kkhan
Could these be added to the registry with some special flag?
My vague thought is that they would then be ‘hidden’ in admin-only mode, unless r-r-d is called with some new parameter.
In normal mode they are visible.

> On 14 Jul 2015, at 20:59, Brian Stansberry <[hidden email]> wrote:
>
> One thing that came up yesterday is generating code based on the model
> description. If you're doing that you want as much data as possible. You
> don't want to bring up an -admin-only process (say one embedded in the
> code generation tool) just to read and generate, and then find out you
> missed a bunch of attributes.
>
> On 7/14/15 1:52 PM, James R. Perkins wrote:
>> If the attribute is going to be undefined what is the benefit of having
>> it in the model? If the tooling is looking for an attribute that doesn't
>> exist wouldn't they just check for the existence with ModelNode.has()?
>>
>> I don't write any tooling so maybe I'm off :) I just don't see the
>> benefit of adding an attribute that will always be UNDEFINED though.
>>
>> On 07/13/2015 02:26 PM, Tomaž Cerar wrote:
>>> Hey guys,
>>>
>>> We had interesting discussion with Brian on
>>> <https://github.com/wildfly/wildfly-core/pull/848#issuecomment-119778986>https://github.com/wildfly/wildfly-core/pull/848#issuecomment-119778986
>>>
>>> about how we register runtime/metric attribute on resources.
>>>
>>> There are many cases where subsystems only register attributes /
>>> resources only when server is booting into normal mode.
>>> but if it server is booted only to "admin mode" all that
>>> runtime/metrics attributes are not registered and as such not seen in
>>> the model.
>>>
>>> Registering runtime attributes only in normal mode can cause that
>>> results of :read-resource-description/read-resource
>>> wouldn't return attributes that are on resources if server is started
>>> in admin mode or even CLI standalone.
>>> Our metadata already provides information if attributes is
>>> runtime/metric/configuration.
>>>
>>> This can cause problems for tooling that relies on output of those two
>>> operations.
>>>
>>> Looking at current state of the code, we do use both ways of
>>> registering attributes either conditionally or always.
>>> This probably originates from times where there was no good api/way to
>>> mark attribute/resource as runtime.
>>>
>>> I am personally am in favor of always registering runtime attributes
>>> as this makes sure that user isn't surprised by some extra/missing
>>> attributes based on fact how it is starting the server.
>>>
>>>
>>> What do you guys think? Should we always register it or have it
>>> conditionally?
>>>
>>> --
>>> tomaz
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> wildfly-dev mailing list
>>> [hidden email]
>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>
>> --
>> James R. Perkins
>> JBoss by Red Hat
>>
>>
>>
>> _______________________________________________
>> wildfly-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>
>
>
> --
> Brian Stansberry
> Senior Principal Software Engineer
> JBoss by Red Hat
> _______________________________________________
> wildfly-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/wildfly-dev


_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Metrics & runtime attribute registration

Brian Stansberry
The registry already has data to distinguish these; they are registered
as metrics.

Heiko Braun had a good point today though. The issues we're discussing
here are not limited to --admin-only. There are many metrics that can be
disabled by setting some "statistics-enabled" attribute somewhere to
false. Those metrics will still be registered though; they'll just not
have a typical value. Since we have to deal with that case anyway, I
don't see much value in hiding the attributes in --admin-only.

Jason and I had a long chat in the WildFly hipchat room today though and
for many metrics, returning null is silly. For example counters, where 0
is a perfectly reasonable value. Which further argues against hiding the
metrics.

If there are 3 categories of metrics:

1) Those with a natural 'initial value' like 0 to use if there is no runtime
2) Those that intuitively might have a null value, like a
'last-request-time' for some handler that's never handled a request
3) Those that don't naturally have a natural 'initial value' and also
aren't intuitively nillable (say a "free-space" metric)

How many are in the 3rd category and how big of a problem is it to
return null anyway?

On 7/14/15 3:33 PM, Kabir Khan wrote:

> Could these be added to the registry with some special flag?
> My vague thought is that they would then be ‘hidden’ in admin-only mode, unless r-r-d is called with some new parameter.
> In normal mode they are visible.
>
>> On 14 Jul 2015, at 20:59, Brian Stansberry <[hidden email]> wrote:
>>
>> One thing that came up yesterday is generating code based on the model
>> description. If you're doing that you want as much data as possible. You
>> don't want to bring up an -admin-only process (say one embedded in the
>> code generation tool) just to read and generate, and then find out you
>> missed a bunch of attributes.
>>
>> On 7/14/15 1:52 PM, James R. Perkins wrote:
>>> If the attribute is going to be undefined what is the benefit of having
>>> it in the model? If the tooling is looking for an attribute that doesn't
>>> exist wouldn't they just check for the existence with ModelNode.has()?
>>>
>>> I don't write any tooling so maybe I'm off :) I just don't see the
>>> benefit of adding an attribute that will always be UNDEFINED though.
>>>
>>> On 07/13/2015 02:26 PM, Tomaž Cerar wrote:
>>>> Hey guys,
>>>>
>>>> We had interesting discussion with Brian on
>>>> <https://github.com/wildfly/wildfly-core/pull/848#issuecomment-119778986>https://github.com/wildfly/wildfly-core/pull/848#issuecomment-119778986
>>>>
>>>> about how we register runtime/metric attribute on resources.
>>>>
>>>> There are many cases where subsystems only register attributes /
>>>> resources only when server is booting into normal mode.
>>>> but if it server is booted only to "admin mode" all that
>>>> runtime/metrics attributes are not registered and as such not seen in
>>>> the model.
>>>>
>>>> Registering runtime attributes only in normal mode can cause that
>>>> results of :read-resource-description/read-resource
>>>> wouldn't return attributes that are on resources if server is started
>>>> in admin mode or even CLI standalone.
>>>> Our metadata already provides information if attributes is
>>>> runtime/metric/configuration.
>>>>
>>>> This can cause problems for tooling that relies on output of those two
>>>> operations.
>>>>
>>>> Looking at current state of the code, we do use both ways of
>>>> registering attributes either conditionally or always.
>>>> This probably originates from times where there was no good api/way to
>>>> mark attribute/resource as runtime.
>>>>
>>>> I am personally am in favor of always registering runtime attributes
>>>> as this makes sure that user isn't surprised by some extra/missing
>>>> attributes based on fact how it is starting the server.
>>>>
>>>>
>>>> What do you guys think? Should we always register it or have it
>>>> conditionally?
>>>>
>>>> --
>>>> tomaz
>>>>
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> wildfly-dev mailing list
>>>> [hidden email]
>>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>>
>>> --
>>> James R. Perkins
>>> JBoss by Red Hat
>>>
>>>
>>>
>>> _______________________________________________
>>> wildfly-dev mailing list
>>> [hidden email]
>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>>
>>
>>
>> --
>> Brian Stansberry
>> Senior Principal Software Engineer
>> JBoss by Red Hat
>> _______________________________________________
>> wildfly-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>


--
Brian Stansberry
Senior Principal Software Engineer
JBoss by Red Hat
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Metrics & runtime attribute registration

Brian Stansberry
In reply to this post by Tomaž Cerar-2
Based on the discussion in this thread and some related chats in
hipchat, I propose the requirements below for this general topic, and
very much welcome comments.

Note that this thread is about metrics, but handling of non-metric
runtime-only attributes and operations all have the same concerns.

I've placed an * next to requirements that involve some code change in
the WildFly Core kernel. If we can reach a consensus around these
requirements, that consensus can become a JIRA to implement the * items.

I. If an Extension's initialize method is executing in a process type
where the extension CAN install runtime services, the extension MUST
register all metrics, other runtime-only attributes, and runtime-only
operations, regardless of whether the process is running in --admin-only
mode.

II. If an Extension's initialize method is executing in a process type
where the extension CANNOT install runtime services, the extension MUST
NOT register any metrics, other runtime-only attributes, or runtime-only
operations.[1] For example, most extensions will not register these when
executing on a HostController.

III.* The ExtensionContext.isRuntimeOnlyRegistrationValid() MUST return
the proper value to tell the extension which of conditions I and II apply.

IV.* The AttributeDefinition class MUST expose a 'public ModelNode
getUndefinedMetricValue()' method and the builders for it MUST expose a
'public void setUndefinedMetricValue(ModelNode value)' method. (See VI.A
below for the purpose of this.)

A.* If assertions are enabled and an AttributeDefinition that is passed
to ManagementResourceRegistration registerReadOnlyAttribute or
registerReadWriteAttribute returns a defined ModelNode from
getUndefinedMetricValue(), an AssertionError MUST be thrown. Non-metrics
cannot provide undefined metric values.

B.* If assertions are enabled and an AttributeDefinition that is passed
to ManagementResourceRegistration registerMetric returns a defined
ModelNode from getDefaultValue(), an AssertionError MUST be thrown.
Metrics cannot provide default values.

C.* If assertions are enabled and an AttributeDefinition that is passed
to ManagementResourceRegistration registerMetric returns a defined
ModelNode from getUndefinedMetricValue() and also returns 'true' from
isAllowNull(), an AssertionError MUST be thrown. This is an inconsistent
setting, as the undefined metric value will ensure the client never sees
an undefined result.

V. The read-attribute handler for a metric or runtime-only attribute
MUST set a result value consistent with the attribute description
regardless of whether the expected runtime services are disabled due to
running in --admin-only or some other configuration option such as a
'statistics-enabled' attribute being set to 'false'. Specifically, if
the handler might not set a defined value in that situation, the
isAllowNull() method from the attribute's AttributeDefinition MUST
return 'true'.

VI. The read-attribute handler for metrics that have a logical defined
value even if runtime services are not present SHOULD set that value as
the result when runtime services are not present. For example, for many
'counter' metrics a value of 0 is a logical defined value.

A. The read-attribute handler for such a metric MAY choose to not set a
result value itself when there are no runtime services, and instead
delegate to the kernel to have the kernel set the result. To configure
such a delegation, the AttributeDefinition for the metric must return a
defined value from its getUndefinedMetricValue() method. (See IV.)

B.* The kernel's handler for read-attribute MUST check if the registered
handler for the attribute has not set a defined result. If it has not,
and the attribute is a metric, and its definition's
getUndefinedMetricValue() method returns a defined value, the kernel
handler must set that value as the result. This behavior MUST occur
regardless of the value of any 'include-defaults' parameter passed to
the hander. The 'include-defaults' parameter is unrelated to metrics.

VII. The read-attribute handler for metrics that do not have a logical
defined value if runtime services are not present SHOULD return an
undefined result and SHOULD NOT return an arbitrary meaningless value
such as -1 or Integer.MAX_VALUE. This is not a MUST/MUST NOT requirement
because it is understood that backwards compatibility requirements may
prevent changes to some attributes, and also that some integrated
libraries may return such values without the subsystem handler's being
aware of that.

VIII. The read-attribute handler for a custom runtime-only operation
MUST set a result value consistent with the operation description
regardless of whether the expected runtime services are disabled due to
running in --admin-only or some other configuration option.
Specifically, if the handler might not set a defined value in that
situation, the isReplyAllowNull() method from the attribute's
OperationDefinition MUST return 'true'.

A. However, it is permissible for a custom runtime-only operation
handler to throw an OperationFailedException in this situation. The
'runtime-only'=>true entry in the operation's descriptive metadata
provides an indication to callers that this is a possible result.

I think that's about it. ;)

[1] At some point in the future the restriction on runtime-only
operations may be lifted, in order to support executing the operation on
all servers in a domain.

On 7/13/15 4:26 PM, Tomaž Cerar wrote:

> Hey guys,
>
> We had interesting discussion with Brian on
> https://github.com/wildfly/wildfly-core/pull/848#issuecomment-119778986
> about how we register runtime/metric attribute on resources.
>
> There are many cases where subsystems only register attributes /
> resources only when server is booting into normal mode.
> but if it server is booted only to "admin mode" all that runtime/metrics
> attributes are not registered and as such not seen in the model.
>
> Registering runtime attributes only in normal mode can cause that
> results of :read-resource-description/read-resource
> wouldn't return attributes that are on resources if server is started in
> admin mode or even CLI standalone.
> Our metadata already provides information if attributes is
> runtime/metric/configuration.
>
> This can cause problems for tooling that relies on output of those two
> operations.
>
> Looking at current state of the code, we do use both ways of registering
> attributes either conditionally or always.
> This probably originates from times where there was no good api/way to
> mark attribute/resource as runtime.
>
> I am personally am in favor of always registering runtime attributes as
> this makes sure that user isn't surprised by some extra/missing
> attributes based on fact how it is starting the server.
>
>
> What do you guys think? Should we always register it or have it
> conditionally?
>
> --
> tomaz
>
>
>
>
> _______________________________________________
> wildfly-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>


--
Brian Stansberry
Senior Principal Software Engineer
JBoss by Red Hat
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Metrics & runtime attribute registration

Brian Stansberry
Silence is assent. ;) So I've opened
https://issues.jboss.org/browse/WFCORE-831.

On 7/15/15 1:26 PM, Brian Stansberry wrote:

> Based on the discussion in this thread and some related chats in
> hipchat, I propose the requirements below for this general topic, and
> very much welcome comments.
>
> Note that this thread is about metrics, but handling of non-metric
> runtime-only attributes and operations all have the same concerns.
>
> I've placed an * next to requirements that involve some code change in
> the WildFly Core kernel. If we can reach a consensus around these
> requirements, that consensus can become a JIRA to implement the * items.
>
> I. If an Extension's initialize method is executing in a process type
> where the extension CAN install runtime services, the extension MUST
> register all metrics, other runtime-only attributes, and runtime-only
> operations, regardless of whether the process is running in --admin-only
> mode.
>
> II. If an Extension's initialize method is executing in a process type
> where the extension CANNOT install runtime services, the extension MUST
> NOT register any metrics, other runtime-only attributes, or runtime-only
> operations.[1] For example, most extensions will not register these when
> executing on a HostController.
>
> III.* The ExtensionContext.isRuntimeOnlyRegistrationValid() MUST return
> the proper value to tell the extension which of conditions I and II apply.
>
> IV.* The AttributeDefinition class MUST expose a 'public ModelNode
> getUndefinedMetricValue()' method and the builders for it MUST expose a
> 'public void setUndefinedMetricValue(ModelNode value)' method. (See VI.A
> below for the purpose of this.)
>
> A.* If assertions are enabled and an AttributeDefinition that is passed
> to ManagementResourceRegistration registerReadOnlyAttribute or
> registerReadWriteAttribute returns a defined ModelNode from
> getUndefinedMetricValue(), an AssertionError MUST be thrown. Non-metrics
> cannot provide undefined metric values.
>
> B.* If assertions are enabled and an AttributeDefinition that is passed
> to ManagementResourceRegistration registerMetric returns a defined
> ModelNode from getDefaultValue(), an AssertionError MUST be thrown.
> Metrics cannot provide default values.
>
> C.* If assertions are enabled and an AttributeDefinition that is passed
> to ManagementResourceRegistration registerMetric returns a defined
> ModelNode from getUndefinedMetricValue() and also returns 'true' from
> isAllowNull(), an AssertionError MUST be thrown. This is an inconsistent
> setting, as the undefined metric value will ensure the client never sees
> an undefined result.
>
> V. The read-attribute handler for a metric or runtime-only attribute
> MUST set a result value consistent with the attribute description
> regardless of whether the expected runtime services are disabled due to
> running in --admin-only or some other configuration option such as a
> 'statistics-enabled' attribute being set to 'false'. Specifically, if
> the handler might not set a defined value in that situation, the
> isAllowNull() method from the attribute's AttributeDefinition MUST
> return 'true'.
>
> VI. The read-attribute handler for metrics that have a logical defined
> value even if runtime services are not present SHOULD set that value as
> the result when runtime services are not present. For example, for many
> 'counter' metrics a value of 0 is a logical defined value.
>
> A. The read-attribute handler for such a metric MAY choose to not set a
> result value itself when there are no runtime services, and instead
> delegate to the kernel to have the kernel set the result. To configure
> such a delegation, the AttributeDefinition for the metric must return a
> defined value from its getUndefinedMetricValue() method. (See IV.)
>
> B.* The kernel's handler for read-attribute MUST check if the registered
> handler for the attribute has not set a defined result. If it has not,
> and the attribute is a metric, and its definition's
> getUndefinedMetricValue() method returns a defined value, the kernel
> handler must set that value as the result. This behavior MUST occur
> regardless of the value of any 'include-defaults' parameter passed to
> the hander. The 'include-defaults' parameter is unrelated to metrics.
>
> VII. The read-attribute handler for metrics that do not have a logical
> defined value if runtime services are not present SHOULD return an
> undefined result and SHOULD NOT return an arbitrary meaningless value
> such as -1 or Integer.MAX_VALUE. This is not a MUST/MUST NOT requirement
> because it is understood that backwards compatibility requirements may
> prevent changes to some attributes, and also that some integrated
> libraries may return such values without the subsystem handler's being
> aware of that.
>
> VIII. The read-attribute handler for a custom runtime-only operation
> MUST set a result value consistent with the operation description
> regardless of whether the expected runtime services are disabled due to
> running in --admin-only or some other configuration option.
> Specifically, if the handler might not set a defined value in that
> situation, the isReplyAllowNull() method from the attribute's
> OperationDefinition MUST return 'true'.
>
> A. However, it is permissible for a custom runtime-only operation
> handler to throw an OperationFailedException in this situation. The
> 'runtime-only'=>true entry in the operation's descriptive metadata
> provides an indication to callers that this is a possible result.
>
> I think that's about it. ;)
>
> [1] At some point in the future the restriction on runtime-only
> operations may be lifted, in order to support executing the operation on
> all servers in a domain.
>
> On 7/13/15 4:26 PM, Tomaž Cerar wrote:
>> Hey guys,
>>
>> We had interesting discussion with Brian on
>> https://github.com/wildfly/wildfly-core/pull/848#issuecomment-119778986
>> about how we register runtime/metric attribute on resources.
>>
>> There are many cases where subsystems only register attributes /
>> resources only when server is booting into normal mode.
>> but if it server is booted only to "admin mode" all that runtime/metrics
>> attributes are not registered and as such not seen in the model.
>>
>> Registering runtime attributes only in normal mode can cause that
>> results of :read-resource-description/read-resource
>> wouldn't return attributes that are on resources if server is started in
>> admin mode or even CLI standalone.
>> Our metadata already provides information if attributes is
>> runtime/metric/configuration.
>>
>> This can cause problems for tooling that relies on output of those two
>> operations.
>>
>> Looking at current state of the code, we do use both ways of registering
>> attributes either conditionally or always.
>> This probably originates from times where there was no good api/way to
>> mark attribute/resource as runtime.
>>
>> I am personally am in favor of always registering runtime attributes as
>> this makes sure that user isn't surprised by some extra/missing
>> attributes based on fact how it is starting the server.
>>
>>
>> What do you guys think? Should we always register it or have it
>> conditionally?
>>
>> --
>> tomaz
>>
>>
>>
>>
>> _______________________________________________
>> wildfly-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>
>
>


--
Brian Stansberry
Senior Principal Software Engineer
JBoss by Red Hat
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Metrics & runtime attribute registration

kkhan
I am looking at https://issues.jboss.org/browse/WFCORE-831. Basically what is new is registering the same ‘stuff’ for an admin-only server, as for a normal server. Does this only apply to attributes?
Having a glance of the usage of EC.isRuntimeOnlyRegistrationValid(), in addition to toggling registration of attributes we currently use is to toggle:

a) Register runtime resources in the main subsystem model model (probably some examples in messaging)
b) Register deployment resources (e.g. logging and datasources)
c) Register runtime operations (e.g. jdr report, Ds flush)

I don't think having b) and c) makes sense if it is an admin-only server, but perhaps a) does? Still, where would the data for a) come from?


> On 20 Jul 2015, at 15:32, Brian Stansberry <[hidden email]> wrote:
>
> Silence is assent. ;) So I've opened
> https://issues.jboss.org/browse/WFCORE-831.
>
> On 7/15/15 1:26 PM, Brian Stansberry wrote:
>> Based on the discussion in this thread and some related chats in
>> hipchat, I propose the requirements below for this general topic, and
>> very much welcome comments.
>>
>> Note that this thread is about metrics, but handling of non-metric
>> runtime-only attributes and operations all have the same concerns.
>>
>> I've placed an * next to requirements that involve some code change in
>> the WildFly Core kernel. If we can reach a consensus around these
>> requirements, that consensus can become a JIRA to implement the * items.
>>
>> I. If an Extension's initialize method is executing in a process type
>> where the extension CAN install runtime services, the extension MUST
>> register all metrics, other runtime-only attributes, and runtime-only
>> operations, regardless of whether the process is running in --admin-only
>> mode.
>>
>> II. If an Extension's initialize method is executing in a process type
>> where the extension CANNOT install runtime services, the extension MUST
>> NOT register any metrics, other runtime-only attributes, or runtime-only
>> operations.[1] For example, most extensions will not register these when
>> executing on a HostController.
>>
>> III.* The ExtensionContext.isRuntimeOnlyRegistrationValid() MUST return
>> the proper value to tell the extension which of conditions I and II apply.
>>
>> IV.* The AttributeDefinition class MUST expose a 'public ModelNode
>> getUndefinedMetricValue()' method and the builders for it MUST expose a
>> 'public void setUndefinedMetricValue(ModelNode value)' method. (See VI.A
>> below for the purpose of this.)
>>
>> A.* If assertions are enabled and an AttributeDefinition that is passed
>> to ManagementResourceRegistration registerReadOnlyAttribute or
>> registerReadWriteAttribute returns a defined ModelNode from
>> getUndefinedMetricValue(), an AssertionError MUST be thrown. Non-metrics
>> cannot provide undefined metric values.
>>
>> B.* If assertions are enabled and an AttributeDefinition that is passed
>> to ManagementResourceRegistration registerMetric returns a defined
>> ModelNode from getDefaultValue(), an AssertionError MUST be thrown.
>> Metrics cannot provide default values.
>>
>> C.* If assertions are enabled and an AttributeDefinition that is passed
>> to ManagementResourceRegistration registerMetric returns a defined
>> ModelNode from getUndefinedMetricValue() and also returns 'true' from
>> isAllowNull(), an AssertionError MUST be thrown. This is an inconsistent
>> setting, as the undefined metric value will ensure the client never sees
>> an undefined result.
>>
>> V. The read-attribute handler for a metric or runtime-only attribute
>> MUST set a result value consistent with the attribute description
>> regardless of whether the expected runtime services are disabled due to
>> running in --admin-only or some other configuration option such as a
>> 'statistics-enabled' attribute being set to 'false'. Specifically, if
>> the handler might not set a defined value in that situation, the
>> isAllowNull() method from the attribute's AttributeDefinition MUST
>> return 'true'.
>>
>> VI. The read-attribute handler for metrics that have a logical defined
>> value even if runtime services are not present SHOULD set that value as
>> the result when runtime services are not present. For example, for many
>> 'counter' metrics a value of 0 is a logical defined value.
>>
>> A. The read-attribute handler for such a metric MAY choose to not set a
>> result value itself when there are no runtime services, and instead
>> delegate to the kernel to have the kernel set the result. To configure
>> such a delegation, the AttributeDefinition for the metric must return a
>> defined value from its getUndefinedMetricValue() method. (See IV.)
>>
>> B.* The kernel's handler for read-attribute MUST check if the registered
>> handler for the attribute has not set a defined result. If it has not,
>> and the attribute is a metric, and its definition's
>> getUndefinedMetricValue() method returns a defined value, the kernel
>> handler must set that value as the result. This behavior MUST occur
>> regardless of the value of any 'include-defaults' parameter passed to
>> the hander. The 'include-defaults' parameter is unrelated to metrics.
>>
>> VII. The read-attribute handler for metrics that do not have a logical
>> defined value if runtime services are not present SHOULD return an
>> undefined result and SHOULD NOT return an arbitrary meaningless value
>> such as -1 or Integer.MAX_VALUE. This is not a MUST/MUST NOT requirement
>> because it is understood that backwards compatibility requirements may
>> prevent changes to some attributes, and also that some integrated
>> libraries may return such values without the subsystem handler's being
>> aware of that.
>>
>> VIII. The read-attribute handler for a custom runtime-only operation
>> MUST set a result value consistent with the operation description
>> regardless of whether the expected runtime services are disabled due to
>> running in --admin-only or some other configuration option.
>> Specifically, if the handler might not set a defined value in that
>> situation, the isReplyAllowNull() method from the attribute's
>> OperationDefinition MUST return 'true'.
>>
>> A. However, it is permissible for a custom runtime-only operation
>> handler to throw an OperationFailedException in this situation. The
>> 'runtime-only'=>true entry in the operation's descriptive metadata
>> provides an indication to callers that this is a possible result.
>>
>> I think that's about it. ;)
>>
>> [1] At some point in the future the restriction on runtime-only
>> operations may be lifted, in order to support executing the operation on
>> all servers in a domain.
>>
>> On 7/13/15 4:26 PM, Tomaž Cerar wrote:
>>> Hey guys,
>>>
>>> We had interesting discussion with Brian on
>>> https://github.com/wildfly/wildfly-core/pull/848#issuecomment-119778986
>>> about how we register runtime/metric attribute on resources.
>>>
>>> There are many cases where subsystems only register attributes /
>>> resources only when server is booting into normal mode.
>>> but if it server is booted only to "admin mode" all that runtime/metrics
>>> attributes are not registered and as such not seen in the model.
>>>
>>> Registering runtime attributes only in normal mode can cause that
>>> results of :read-resource-description/read-resource
>>> wouldn't return attributes that are on resources if server is started in
>>> admin mode or even CLI standalone.
>>> Our metadata already provides information if attributes is
>>> runtime/metric/configuration.
>>>
>>> This can cause problems for tooling that relies on output of those two
>>> operations.
>>>
>>> Looking at current state of the code, we do use both ways of registering
>>> attributes either conditionally or always.
>>> This probably originates from times where there was no good api/way to
>>> mark attribute/resource as runtime.
>>>
>>> I am personally am in favor of always registering runtime attributes as
>>> this makes sure that user isn't surprised by some extra/missing
>>> attributes based on fact how it is starting the server.
>>>
>>>
>>> What do you guys think? Should we always register it or have it
>>> conditionally?
>>>
>>> --
>>> tomaz
>>>
>>>
>>>
>>>
>>> _______________________________________________
>>> wildfly-dev mailing list
>>> [hidden email]
>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>>
>>
>>
>
>
> --
> Brian Stansberry
> Senior Principal Software Engineer
> JBoss by Red Hat
> _______________________________________________
> wildfly-dev mailing list
> [hidden email]
> https://lists.jboss.org/mailman/listinfo/wildfly-dev


_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev
Reply | Threaded
Open this post in threaded view
|

Re: Metrics & runtime attribute registration

Brian Stansberry
The basic thrust of this thread was that the *API* should not be
changing based on things like --admin-only or statistics-enabled=false.
The results obtained by invoking ops can change, though, so long as
those results are consistent with the published metadata.

So, I don't see a problem with c). VII.A. in the WFCORE-831 description
covers (admittedly in a not-so-elegant way) how the results obtained can
change:

"However, it is permissible for a custom runtime-only operation handler
to throw an OperationFailedException in this situation. The
'runtime-only'=>true entry in the operation's descriptive metadata
provides an indication to callers that this is a possible result."

Where this starts to break down is if the API promises that a particular
resource will exist, and it just can't. Carrying the WFCORE-831
semantics for metrics all the way forward to creating fictitious
resources is too far.

If a child runtime-only resource is registered under foo=* that's not
such a problem as there's no promise that any particular value of "*"
will exist. But /deployment=x.war/subsystem=logging is more of a problem.

Extending the VIII.A type thing to a child resource (i.e so throwing
NoSuchResourceException is ok, because the resource is 'runtime-only'
sounds bad. People invoke custom ops like 'flush-ds' intentionally. But
trying to walk a resource tree is something much more likely to be done
automatically by a tool.

How forgiving is the read-resource handler? IIRC there's logic in there
to ignore some sorts of runtime-only failures, to avoid their causing
recursive reads to fail.

On 8/24/15 8:38 AM, Kabir Khan wrote:

> I am looking at https://issues.jboss.org/browse/WFCORE-831. Basically what is new is registering the same ‘stuff’ for an admin-only server, as for a normal server. Does this only apply to attributes?
> Having a glance of the usage of EC.isRuntimeOnlyRegistrationValid(), in addition to toggling registration of attributes we currently use is to toggle:
>
> a) Register runtime resources in the main subsystem model model (probably some examples in messaging)
> b) Register deployment resources (e.g. logging and datasources)
> c) Register runtime operations (e.g. jdr report, Ds flush)
>
> I don't think having b) and c) makes sense if it is an admin-only server, but perhaps a) does? Still, where would the data for a) come from?
>
>
>> On 20 Jul 2015, at 15:32, Brian Stansberry <[hidden email]> wrote:
>>
>> Silence is assent. ;) So I've opened
>> https://issues.jboss.org/browse/WFCORE-831.
>>
>> On 7/15/15 1:26 PM, Brian Stansberry wrote:
>>> Based on the discussion in this thread and some related chats in
>>> hipchat, I propose the requirements below for this general topic, and
>>> very much welcome comments.
>>>
>>> Note that this thread is about metrics, but handling of non-metric
>>> runtime-only attributes and operations all have the same concerns.
>>>
>>> I've placed an * next to requirements that involve some code change in
>>> the WildFly Core kernel. If we can reach a consensus around these
>>> requirements, that consensus can become a JIRA to implement the * items.
>>>
>>> I. If an Extension's initialize method is executing in a process type
>>> where the extension CAN install runtime services, the extension MUST
>>> register all metrics, other runtime-only attributes, and runtime-only
>>> operations, regardless of whether the process is running in --admin-only
>>> mode.
>>>
>>> II. If an Extension's initialize method is executing in a process type
>>> where the extension CANNOT install runtime services, the extension MUST
>>> NOT register any metrics, other runtime-only attributes, or runtime-only
>>> operations.[1] For example, most extensions will not register these when
>>> executing on a HostController.
>>>
>>> III.* The ExtensionContext.isRuntimeOnlyRegistrationValid() MUST return
>>> the proper value to tell the extension which of conditions I and II apply.
>>>
>>> IV.* The AttributeDefinition class MUST expose a 'public ModelNode
>>> getUndefinedMetricValue()' method and the builders for it MUST expose a
>>> 'public void setUndefinedMetricValue(ModelNode value)' method. (See VI.A
>>> below for the purpose of this.)
>>>
>>> A.* If assertions are enabled and an AttributeDefinition that is passed
>>> to ManagementResourceRegistration registerReadOnlyAttribute or
>>> registerReadWriteAttribute returns a defined ModelNode from
>>> getUndefinedMetricValue(), an AssertionError MUST be thrown. Non-metrics
>>> cannot provide undefined metric values.
>>>
>>> B.* If assertions are enabled and an AttributeDefinition that is passed
>>> to ManagementResourceRegistration registerMetric returns a defined
>>> ModelNode from getDefaultValue(), an AssertionError MUST be thrown.
>>> Metrics cannot provide default values.
>>>
>>> C.* If assertions are enabled and an AttributeDefinition that is passed
>>> to ManagementResourceRegistration registerMetric returns a defined
>>> ModelNode from getUndefinedMetricValue() and also returns 'true' from
>>> isAllowNull(), an AssertionError MUST be thrown. This is an inconsistent
>>> setting, as the undefined metric value will ensure the client never sees
>>> an undefined result.
>>>
>>> V. The read-attribute handler for a metric or runtime-only attribute
>>> MUST set a result value consistent with the attribute description
>>> regardless of whether the expected runtime services are disabled due to
>>> running in --admin-only or some other configuration option such as a
>>> 'statistics-enabled' attribute being set to 'false'. Specifically, if
>>> the handler might not set a defined value in that situation, the
>>> isAllowNull() method from the attribute's AttributeDefinition MUST
>>> return 'true'.
>>>
>>> VI. The read-attribute handler for metrics that have a logical defined
>>> value even if runtime services are not present SHOULD set that value as
>>> the result when runtime services are not present. For example, for many
>>> 'counter' metrics a value of 0 is a logical defined value.
>>>
>>> A. The read-attribute handler for such a metric MAY choose to not set a
>>> result value itself when there are no runtime services, and instead
>>> delegate to the kernel to have the kernel set the result. To configure
>>> such a delegation, the AttributeDefinition for the metric must return a
>>> defined value from its getUndefinedMetricValue() method. (See IV.)
>>>
>>> B.* The kernel's handler for read-attribute MUST check if the registered
>>> handler for the attribute has not set a defined result. If it has not,
>>> and the attribute is a metric, and its definition's
>>> getUndefinedMetricValue() method returns a defined value, the kernel
>>> handler must set that value as the result. This behavior MUST occur
>>> regardless of the value of any 'include-defaults' parameter passed to
>>> the hander. The 'include-defaults' parameter is unrelated to metrics.
>>>
>>> VII. The read-attribute handler for metrics that do not have a logical
>>> defined value if runtime services are not present SHOULD return an
>>> undefined result and SHOULD NOT return an arbitrary meaningless value
>>> such as -1 or Integer.MAX_VALUE. This is not a MUST/MUST NOT requirement
>>> because it is understood that backwards compatibility requirements may
>>> prevent changes to some attributes, and also that some integrated
>>> libraries may return such values without the subsystem handler's being
>>> aware of that.
>>>
>>> VIII. The read-attribute handler for a custom runtime-only operation
>>> MUST set a result value consistent with the operation description
>>> regardless of whether the expected runtime services are disabled due to
>>> running in --admin-only or some other configuration option.
>>> Specifically, if the handler might not set a defined value in that
>>> situation, the isReplyAllowNull() method from the attribute's
>>> OperationDefinition MUST return 'true'.
>>>
>>> A. However, it is permissible for a custom runtime-only operation
>>> handler to throw an OperationFailedException in this situation. The
>>> 'runtime-only'=>true entry in the operation's descriptive metadata
>>> provides an indication to callers that this is a possible result.
>>>
>>> I think that's about it. ;)
>>>
>>> [1] At some point in the future the restriction on runtime-only
>>> operations may be lifted, in order to support executing the operation on
>>> all servers in a domain.
>>>
>>> On 7/13/15 4:26 PM, Tomaž Cerar wrote:
>>>> Hey guys,
>>>>
>>>> We had interesting discussion with Brian on
>>>> https://github.com/wildfly/wildfly-core/pull/848#issuecomment-119778986
>>>> about how we register runtime/metric attribute on resources.
>>>>
>>>> There are many cases where subsystems only register attributes /
>>>> resources only when server is booting into normal mode.
>>>> but if it server is booted only to "admin mode" all that runtime/metrics
>>>> attributes are not registered and as such not seen in the model.
>>>>
>>>> Registering runtime attributes only in normal mode can cause that
>>>> results of :read-resource-description/read-resource
>>>> wouldn't return attributes that are on resources if server is started in
>>>> admin mode or even CLI standalone.
>>>> Our metadata already provides information if attributes is
>>>> runtime/metric/configuration.
>>>>
>>>> This can cause problems for tooling that relies on output of those two
>>>> operations.
>>>>
>>>> Looking at current state of the code, we do use both ways of registering
>>>> attributes either conditionally or always.
>>>> This probably originates from times where there was no good api/way to
>>>> mark attribute/resource as runtime.
>>>>
>>>> I am personally am in favor of always registering runtime attributes as
>>>> this makes sure that user isn't surprised by some extra/missing
>>>> attributes based on fact how it is starting the server.
>>>>
>>>>
>>>> What do you guys think? Should we always register it or have it
>>>> conditionally?
>>>>
>>>> --
>>>> tomaz
>>>>
>>>>
>>>>
>>>>
>>>> _______________________________________________
>>>> wildfly-dev mailing list
>>>> [hidden email]
>>>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>>>>
>>>
>>>
>>
>>
>> --
>> Brian Stansberry
>> Senior Principal Software Engineer
>> JBoss by Red Hat
>> _______________________________________________
>> wildfly-dev mailing list
>> [hidden email]
>> https://lists.jboss.org/mailman/listinfo/wildfly-dev
>


--
Brian Stansberry
Senior Principal Software Engineer
JBoss by Red Hat
_______________________________________________
wildfly-dev mailing list
[hidden email]
https://lists.jboss.org/mailman/listinfo/wildfly-dev