Skip to content
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

Feature Request: Add a way to get the last measurement #825

Open
carl-mastrangelo opened this issue Jun 10, 2020 · 2 comments
Open

Feature Request: Add a way to get the last measurement #825

carl-mastrangelo opened this issue Jun 10, 2020 · 2 comments

Comments

@carl-mastrangelo
Copy link

When unit testing that counters, gauges, and other meters are correctly updated, it's verbose to get the latest numbers. I think it would be useful to provide a way to get the last measurement value on a meter. Currently, I call Meter.measure, sort the entries by timestamp, and pick the last one.

One way to fix this would be a lastMeasurement default method on the interface, that does effectively the same thing. Individual implementations that keep an ordered list of measurements can override it and provide an optimized variant. This would allow me to build an "expected" and "actual" measurement map, from Id to Double. I could compare the two maps to ensure they match in my tests.

@brharrington
Copy link
Contributor

I'm not sure I follow the exact problem your having. Do you have a sample unit test showing the issue?

There are some docs on testing: https://netflix.github.io/spectator/en/latest/intro/testing/

@carl-mastrangelo
Copy link
Author

I do, though it's internal atm. The gist of it is:

        Map<Id, Double> expectedMetrics = Map.of(
                registry.createId("some.id.1", "bucket", "bucket", "key", expectedKey1, "success", "true"), 1D,
                registry.createId("some.id.2", "bucket", "bucket", "key", expectedKey2, "success", "true"), 1D,
                registry.createId("some.id.3", "bucket", "bucket", "key", expectedKey3, "success", "true"), 1D,
                registry.createId("some.id.4", "bucket", "bucket", "count", "3", "success", "true"), 1D,
                registry.createId("some.id.5"), 2D,
                registry.createId("some.id.6", "key", expectedKey4, "status", "OK"), 1D);

        runTheTest(registry);

        Truth.assertThat(buildMap(registry)).containsExactlyEntriesIn(expectedMetrics);

Build Map looks like:

    private static Map<Id, Double> buildMap(Registry registry) {
        Map<Id, Double> actualMeasurements = new LinkedHashMap<>();
        for (Meter m : registry) {
            Measurement last = null;
            for (Measurement ms : m.measure()) {
                if (last == null || ms.timestamp() > last.timestamp()) {
                    last = ms;
                }
            }
            actualMeasurements.put(m.id(), Objects.requireNonNull(last).value());
        }
        return Collections.unmodifiableMap(actualMeasurements);
    }

The problem with this approach is that's verbose. Calling registry.count(id) is more difficult, because not all the meters are counters. Splitting each each type out risks forgetting about a certain kind of meter. By comparing the entire registry to an expected registry, I can confirm no metrics are accidentally not accounted for.

Providing a lastMeasurement() would allow me to get right of buildMap function, and make a stream based map.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants