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

Leverage the fact that Id could have a reference to the registry #440

Open
quidryan opened this issue Aug 3, 2017 · 1 comment
Open

Comments

@quidryan
Copy link
Contributor

quidryan commented Aug 3, 2017

As a user, I would like to hold references to just Id's. Currently I have to keep a reference to the registry, in the cases where my Id's are dynamic based on the data, e.g
In my constructor, I would do establish a base Id and have to keep a registry around:

    private final Registry registry;
    private final Id RESPONSE_ANALYZE_ID;

    public ResponseAnalyzeService(Registry registry) {
        this.registry = registry;
        this.RESPONSE_ANALYZE_ID = registry.createId("response.analyze");
    }

Then in the main code path:

        Id id = null;
        try {
            ResponseAnalysis responseAnalysis = analyzeResponseRaw(requestState);
            id = RESPONSE_ANALYZE_ID
                        .withTag("success", "true")
                        .withTag("action", responseAnalysis.getAction().name())
                        .withTag("reason", responseAnalysis.getReason().name());
        } catch(Exception e) {
            id = RESPONSE_ANALYZE_ID
                    .withTag("success", "false")
                    .withTag("exception", e.getClass().getSimpleName());
        } finally {
            if (id != null) {
                registry.counter(id).increment();
            }
        }

There isn't currently a way to establish a counter based on the dynamic Id, without keeping a reference to the registry around. Since the original RESPONSE_ANALYZE_ID Id in this example came from a registry, it doesn't seem far fetched that it could have a reference to the registry, that would allow some call like id.counter().increment().

I get that it's a big ask to be adding memory references to lightweight objects. I guess my big goal to be able to reduce some of the code in the example, since we follow this pattern a lot and it is more verbose than we would hope and more than the legacy Servo wrapper we have used in the past.

@brharrington
Copy link
Contributor

Thanks for the feedback, I'll have to think about it a bit.

For this particular example since the base id is just the name, you could do something like:

    private static final String RESPONSE_ANALYZE_ID = "response.analyze";
    private final Registry registry;

    public ResponseAnalyzeService(Registry registry) {
        this.registry = registry;
    }

...

        Map<String, String> tags = new HashMap<>();
        try {
            ResponseAnalysis responseAnalysis = analyzeResponseRaw(requestState);
            tags.put("success", "true");
            tags.put("action", responseAnalysis.getAction().name());
            tags.put("reason", responseAnalysis.getReason().name());
        } catch(Exception e) {
            tags.put("success", "false");
            tags.put("exception", e.getClass().getSimpleName());
        } finally {
            registry.counter(registry.createId(RESPONSE_ANALYZE_ID, tags)).increment();
        }

... OR ...

        try {
            ResponseAnalysis responseAnalysis = analyzeResponseRaw(requestState);
            registry.counter(RESPONSE_ANALYZE_ID,
                "success", "true",
                "action", responseAnalysis.getAction().name(),
                "reason", responseAnalysis.getReason().name()).increment();
        } catch(Exception e) {
            registry.counter(RESPONSE_ANALYZE_ID,
                "success", "false",
                "exception", e.getClass().getSimpleName()).increment();
        }

I think the second is the closest to the servo DynamicCounter usage.

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

No branches or pull requests

2 participants