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: metadata for tags #18

Open
mgravell opened this issue Apr 17, 2015 · 11 comments
Open

Feature request: metadata for tags #18

mgravell opened this issue Apr 17, 2015 · 11 comments

Comments

@mgravell
Copy link
Collaborator

I couldn't see an API to describe metadata for tags; does this exist? For example:

image

@bretcope
Copy link
Contributor

All metadata descriptions are sent with the tag values of the metric. When you create a metric group, it will use the description provided to the group as the default description for all of the metric it creates. However, the Description property is public settable, and that is generally what is used for the metadata description.

I just added an example of altering the description on individual metrics from a group. And when metadata is sent for those, it shows up in Bosun like this:

Strictly speaking, the return value of BosunMetric.GetDescription is what is used. The only place that's overridden internally is for AggregateGauge.GetDescription since it reports slightly different descriptions per suffix.

I think that answers your question. If not, or there are other issues, feel free to reopen.

@mgravell
Copy link
Collaborator Author

In my usage I have multiple dimensions on the tags. Does the example still
apply then? How would it know which tags I'm describing in the description?
On 17 Apr 2015 18:47, "Bret Copeland" [email protected] wrote:

Closed #18 #18.


Reply to this email directly or view it on GitHub
#18 (comment).

@bretcope
Copy link
Contributor

There isn't currently a way to describe just certain tags. My understanding was that that wasn't the way metadata was supposed to be specified. Let's talk next week about the best way to implement it, if it needs to be implemented.

@bretcope bretcope reopened this Apr 17, 2015
@mgravell
Copy link
Collaborator Author

We should drag Kyle into that discussion; this is indirectly his request

On 17 April 2015 at 19:32, Bret Copeland [email protected] wrote:

Reopened #18 #18.


Reply to this email directly or view it on GitHub
#18 (comment).

Regards,

Marc

@bretcope
Copy link
Contributor

I have time to revisit this now, if necessary.

@lockwobr
Copy link
Contributor

I would like this if its possible.

@bretcope
Copy link
Contributor

@lockwobr do you have thoughts on what an implementation looks like?

@lockwobr
Copy link
Contributor

well I have an idea for part of it, add a description to BosunTag attribute.

public class RouteCounter : Counter
{
    [BosunTag("Optional Description")] 
    public readonly string Route;

    [BosunTag("Optional Description")] 
    public readonly string Result;

    public RouteCounter(string route, bool ok)
    {
        Route = route;
        Result = ok ? "ok" : "error";
    }
}

I am not sure how this would work out for the metric groups. Maybe doing some with more of a key/value pair would be a better way to get this to work.

@bretcope
Copy link
Contributor

The problem is that the meta data is always key/value, not just key, so I don't think a simple test attribute would work.

I think the most ideal solution would be to support descriptions on enum values.

public enum SampleResult
{
    [Description("The result was good.")]
    Ok,
    [Description("The result was bad.")]
    Error,
}

public class SampleCounter : Counter
{
    [BosunTag] public readonly SampleResult Result;

    public SampleCounter(SampleResult result)
    {
        Result = result;
    }
}

// ...

var group = collector.GetMetricGroup<SampleResult, SampleCounter>(
                                "sample.counter",
                                "samples",
                                "It counts something.")

group.PopulateFromEnum();

Then BosunReporter would send descriptions for each tag value individually:

  • sample.counter
    • {}: It counts something.
    • {"result":"Ok"}: The result was good.
    • {"result":"Error"}: The result was bad.

This would break with current behavior a bit, because currently BosunReporter would send "It counts something." for each combination of tags/values, which would conflict with the individual descriptions in this case.

What I'm proposing above would be to send the metric description with no tags, but that isn't without problems either, because you could provide different descriptions to different metrics of the same name, and it would be unclear which description to use. Right now it's not a problem because those descriptions are sent with tags to differentiate them.

See the problem? It's very non-trivial to get right, and I'm not sure I have the answers yet.

There's also the problem that, even though using enums is a much better practice than using strings, there may be some situations where enums are impractical. In that case, you would need another way of providing the descriptions (possibly a factory, or just a method to manually define them).

@lockwobr
Copy link
Contributor

I just re-read the feature request, so you have a work around for the feature? But it does not work for all cases? and is not user friendly?

What if you forced the user to create tags and description ahead of time? and otherwise it works as it does now?

@bretcope
Copy link
Contributor

I'm honestly not sure what workaround you're referring to, or what you mean by "What if you forced the user to create tags and description ahead of time?" Can you illustrate with code?

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

3 participants