-
Notifications
You must be signed in to change notification settings - Fork 141
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add multi attribute support #137
Add multi attribute support #137
Conversation
* Add Support for multiple object names & add test for it
jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/InstrumentHelper.groovy
Outdated
Show resolved
Hide resolved
jmx-metrics/src/test/java/io/opentelemetry/contrib/jmxmetrics/InstrumenterHelperTest.java
Show resolved
Hide resolved
jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/MBeanHelper.groovy
Outdated
Show resolved
Hide resolved
jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/InstrumentHelper.groovy
Outdated
Show resolved
Hide resolved
jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/InstrumentHelper.groovy
Outdated
Show resolved
Hide resolved
@Mrod1598 thanks for this. I think the implementation seems ok so it would be helpful to update the docstrings and readme to advertise the new signature(s) along with the changes. |
I'm wording if it's redundant to add all new signatures for all of the signatures added. Do you think it's fine to add a note about here that says that attributes can be swapped out with the |
I think it'd preferable to follow the existing pattern of the showing the instrument signature variations and introduce the new mbean attribute label function map with an example like is done with the labelFuncs. I think the whole thing could be done in 5 lines new or so. In cases where you'd like to share instrument names while creating datapoints for multiple MBean attributes...
- `otel.instrument(MBeanHelper mBeanHelper, String instrumentName, String description, String unit, Map<String, Closure> labelFuncs, Map<String, Map<String, Closure>> attributeLabelFuncs, Closure instrument)`
<... description and example of attributeLabelFuncs with intended purpose... >
`otel.instrument()` provides additional signatures to allow this more expressive MBean attribute access:
- `otel.instrument(MBeanHelper mBeanHelper, String name, String description, String unit, Map<String, Map<String, Closure>> attributeLabelFuncs, Closure instrument)` - `labelFuncs` are empty map.
- `otel.instrument(MBeanHelper mBeanHelper, String name, String description, Map<String, Map<String, Closure>> attributeLabelFuncs, Closure instrument)` - `unit` is "1" and `labelFuncs` are empty map.
- `otel.instrument(MBeanHelper mBeanHelper, String name, Map<String, Map<String, Closure>> attributeLabelFuncs, Closure instrument)` - `description` is empty string, `unit` is "1" and `labelFuncs` are empty map
|
jmx-metrics/src/main/groovy/io/opentelemetry/contrib/jmxmetrics/OtelHelper.groovy
Outdated
Show resolved
Hide resolved
Co-authored-by: Ryan Fitzpatrick <rmfitzpatrick@users.noreply.github.com>
Co-authored-by: Ryan Fitzpatrick <rmfitzpatrick@users.noreply.github.com>
Description:
Adds support for multiple attributes through expanding on the
otel.instrument
signatures.Existing Issue(s):
#136
Testing:
Added to InstrumentHelperTest