You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by "demery-pivotal (GitHub)" <gi...@apache.org> on 2019/02/08 20:12:44 UTC
[GitHub] [geode] demery-pivotal opened pull request #3180:
GEODE-6295: Add Micrometer-based metrics system
This PR supersedes https://github.com/apache/geode/pull/3153, which has been closed.
This PR adds a Micrometer-based metrics system to Geode. The key elements are:
- A `MeterRegistry` that maintains the set of meters for a member. The registry is configured at startup, and gives each meter it creates a set of tags that identify the member and the distributed system.
- A `MemberMetricsSession` manages the configuration and lifecycle of a member's meter registry. This class implements `MetricsSession` interface, which provides methods to connect user-implemented "downstream" registries to publish the member registry's meters to external monitoring systems.
This intent of this limited implementation is to provide the foundation to:
- Instrument Geode code using named, dimensional meters.
- Experiment with a variety of existing monitoring agents that can publish Micrometer-based metrics to external monitoring systems.
- Explore a variety of mechanisms for connecting external monitoring agents.
Some key limitations of this implementation:
- It adds no meters to Geode. It provides a *registry* to which meters can be added.
- It provides a single, limited mechanism to connect external monitoring agents. This limited implementation is designed specifically to support exploring a variety of external monitoring agents.
This implementation makes the `MeterRegistry` and the `MetricsSession` available via methods on `InternalDistributedSystem`. These methods will be moved to an appropriate non-internal interface or class in the future. Likewise, `MetricsSession`, which is currently declared in an internal package, will be moved to an appropriate non-internal package in the future. `MemberMetricsSession` will remain internal.
This implementation *neither addresses nor precludes* additional mechanisms for configuring and connecting monitoring agents. We are considering several mechanisms for possible future implementation:
- Supply a downstream registry at startup via a `CacheFactory`.
- Supply a downstream registry at startup via a service loader.
- Configure and manage a downstream registry at runtime via user-defined Geode extensions.
- Other options TBD.
Thank you for submitting a contribution to Apache Geode.
In order to streamline the review of the contribution we ask you
to ensure the following steps have been taken:
### For all changes:
- [X] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [X] Has your PR been rebased against the latest commit within the target branch (typically `develop`)?
- [X] Is your initial contribution a single, squashed commit?
- [X] Does `gradlew build` run cleanly?
- [X] Have you written or updated unit tests to verify your changes?
- [X] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
### Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and
submit an update to your PR as soon as possible. If you need help, please send an
email to dev@geode.apache.org.
[ Full content available at: https://github.com/apache/geode/pull/3180 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] kirklund commented on issue #3180: GEODE-6295: Add
Micrometer-based metrics system
Posted by "kirklund (GitHub)" <gi...@apache.org>.
Most of these failures are caused by DistributedSystem.connect finding an existing DS connection. The connect then checks to ensure that the existing DS connection is compatible, but this check fails with:
IllegalStateException: A connection to a distributed system already exists in this VM. It has the following configuration: ack-severe-alert-threshold="0"
I think we may have introduced a bug involving DistributionConfigImpl, and it seems to involve the value for ack-severe-alert-threshold. Edit: I think ack-severe-alert-threshold is a red-herring -- I think someone broke the formatting so that there's a missing line-feed before the 1st gemfire.property being printed out.
[ Full content available at: https://github.com/apache/geode/pull/3180 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] demery-pivotal closed pull request #3180:
GEODE-6295: Add Micrometer-based metrics system
Posted by "demery-pivotal (GitHub)" <gi...@apache.org>.
[ pull request closed by demery-pivotal ]
[ Full content available at: https://github.com/apache/geode/pull/3180 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] kirklund commented on issue #3180: GEODE-6295: Add
Micrometer-based metrics system
Posted by "kirklund (GitHub)" <gi...@apache.org>.
Most of these failures are caused by DistributedSystem.connect finding an existing DS connection. The connect then checks to ensure that the existing DS connection is compatible, but this check fails with:
IllegalStateException: A connection to a distributed system already exists in this VM. It has the following configuration: ack-severe-alert-threshold="0"
I think we may have introduced a bug involving DistributionConfigImpl, and it seems to involve the value for ack-severe-alert-threshold.
[ Full content available at: https://github.com/apache/geode/pull/3180 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] kirklund commented on issue #3180: GEODE-6295: Add
Micrometer-based metrics system
Posted by "kirklund (GitHub)" <gi...@apache.org>.
Most of these failures are caused by DistributedSystem.connect finding an existing DS connection. The connect then checks to ensure that the existing DS connection is compatible, but this check fails with:
IllegalStateException: A connection to a distributed system already exists in this VM. It has the following configuration: ack-severe-alert-threshold="0"
I think we may have introduced a bug involving DistributionConfigImpl, and it seems to involve the value for ack-severe-alert-threshold. Edit: I think ack-severe-alert-threshold is a red-herrying -- I think someone broke the formatting so that there's a missing line-feed before the 1st gemfire.property being printed out.
[ Full content available at: https://github.com/apache/geode/pull/3180 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org
[GitHub] [geode] demery-pivotal commented on issue #3180: GEODE-6295:
Add Micrometer-based metrics system
Posted by "demery-pivotal (GitHub)" <gi...@apache.org>.
Changing approach: CacheFactory will add Micrometer registry to cache, and use a service loader to discover metrics endpoints.
[ Full content available at: https://github.com/apache/geode/pull/3180 ]
This message was relayed via gitbox.apache.org for notifications@geode.apache.org