aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.68k stars 3.93k forks source link

Support for CloudWatch InsightRule as a Higher Level Construct #6255

Open iph opened 4 years ago

iph commented 4 years ago

In 2019, CloudWatch announced a new product called "Contributor Insights". It allows you to see high cardinality Top-N data of things like:

or anything that you configure from your logs. It's truly amazing. I see that it exists as a Cloudformation resource and an autogenerated resource

Insights break a lot of the previous paradigms though:

Use Case

I use Contributor Insights and want to put it in dashboards at a higher level than the autogenerated resource :)

Proposed Solution

I need to take a look a bit more and I kinda want to use this issue as the place for that discussion.

There are a lot of things that don't jive well if I were to make this a Metric...or even an IMetric. As an example, the current implementation of statistic works well with the superset of both {Metric, Insight} but if I were to add the 2 new statistics mentioned above, you would have 2 statistics that only are valid on an Insight, which I assume most people don't use today. That would be...odd. Though technically it is a statistic.. ¯\(ツ)

This goes down the rabbit hole. Like metrics have namespaces, dimensions, all this stuff that insights do not have. It makes me wonder "is this really a metric". Answer is probably not, so we would need to create something else that would work well with Dashboards/Alarms that's "similar to a metric but not a metric".

Other


This is a :rocket: Feature Request

rix0rrr commented 4 years ago

IMetric now returns a union-like object, so I guess we can always add a new alternative to the union, but that's not a great experience.

Could you make a table of which type of object can be used where? Can insights be used in Math Expressions? Can Math Expressions be used in Insights?

What is even the difference, because in the docs page you linked to it sounds like they are "just" a Math Expression function.

DRNagar commented 4 years ago

Is there an update on this?

swiftyspiffy commented 4 years ago

This would be a really neat feature to have 😀

arnulfojr commented 3 years ago

Any updates on this?

iph commented 3 years ago

Okay, so after doing some experimentation, we have some high level bits of:

RuleBody is a bit of a mine field. It has at least the schema, which requires a Name and a Version.

From there, there are many different optional fields based on the schema. There is one schema released, but knowing cloudwatch, that is bound to change :)

We could potentially do something like:

export interface RuleProps {
   RuleName: string;
   RuleState?: State; 
   RuleBody: IInsightSchema;
}

export enum State {
    ENABLED = 'ENABLED',
    DISABLED = 'DISABLED'
}

export interface IInsightSchema {
    schema(): Schema;
    body(): any
}

export interface Schema {
   Name: string;
   Version: string;
}

I don't know enough about Typescript here to be like "any is cool", but as of right now I have seen examples where the entirety of the json body is almost completely different. This would allow the implementor to have full flexibility in the RuleBody (we would just inject whatever structure they have), and then we would implement the CloudWatchLogRule structure for customers to use. The end might look something like:

let rule = new InsightRule(this, 'Insighty', {
  RuleName: 'hello-world',
  RuleBody: new CloudWatchLogRule({
     LogGroupNames: ['API-Gateway-Access-Logs*', 'Log-group-name2',
     LogFormat: CloudWatchLogRule.Format.JSON,
     Contribution: {
         Keys: [ "$.ip"],
         ValueOf: "$.requestBytes",
         Filters: [ new CloudWatchLogRule.MatchFilter("$.httpMethod", { In: ["PUT"] } ]
     },
     AggregateOn: "Sum"
  })
});

If you later want to use the math expression, then rule would have methods for the graphs it can generate.

e.g. using the rule above:

...
        new GraphWidget({
                title: 'Graph max datapoint over contributors or something, I duno it's an example',
                width: 12,
                height: 6,
                left: [
                    new cw.MathExpression({
                        expression: '(e1/e2)*100',
                        usingMetrics: {
                            e1: new cw.MathExpression({
                                expression: `${rule.UniqueContributorsMetric()}`,
                                period: Duration.seconds(60),
                                usingMetrics: {}
                            }),
                            e2: new cw.MathExpression({
                                expression: `${rule. MaxContributorValue()}`,
                                period: Duration.seconds(60),
                                usingMetrics: {}
                            })
                        },
                        label: 'Something',
                        period: Duration.seconds(60)
                    })]
            }),

or something. Essentially it will produce the syntax: INSIGHT_RULE_METRIC(rule_name, metricName) pulled from here: https://docs.aws.amazon.com/AmazonCloudWatch/latest/monitoring/ContributorInsights-GraphReportData.html

This is a starting point. Going to iterate a bit on this, but it allows flexibility -- which CW will 100% appreciate (as they consistently add little knobs that always makes it interesting to branch features / options at)

iph commented 3 years ago

Also, for graphing this fella on it's own, we would need to propose a new concrete widget. I wrote one for my own service that looks like this to get an idea of what it looks like:

import { ConcreteWidget } from 'aws-cdk/aws-cloudwatch';

export interface InsightProps {
    width: number;
    height: number;
    ruleName: string;
    region: string;

    maxContributorCount?: number;
    title?: string;
    legendPosition?: string;

}

export class SimpleInsightWidget extends ConcreteWidget {
    private readonly ruleName: string;
    private readonly region: string;

    private readonly maxContributorCount?: number;
    private readonly title?: string;
    private readonly legendPosition?: string;

    constructor(props: InsightProps) {
        super(props.width, props.height);
        this.ruleName = props.ruleName;
        this.region = props.region;
        this.maxContributorCount = props.maxContributorCount;
        this.title = props.title;
        this.legendPosition = props.legendPosition;
    }
    /* tslint:disable:no-any */
    public toJson(): any[] {
        return [{
            type: 'metric',
            x: this.x,
            y: this.y,
            width: this.width,
            height: this.height,
            properties: {
               period: 60,
               insightRule: {
                   maxContributorCount: this.maxContributorCount?? 20,
                   ruleName: this.ruleName
               },
                stacked: false,
                view: 'timeSeries',
                title: this.title ?? this.ruleName,
                legend: {
                   position: this.legendPosition?? 'right'
                },
                region: this.region
            }
        }];
    }

}

Usage would look something like:

        exampleDash.addWidgets(
            new SimpleInsightWidget({
                ruleName: insightRule.name(),
                region: this.region,
                height: 6,
                width: 24
            }),
        );
NetaNir commented 3 years ago

Hi @iph!

I agree that we shouldn't force this into a Metric and instead create a new InsightRule type. As for the implementation, I suggest we do it in phases, lets start with implementing the building blocks, then work backward from some rules and use cases.

For the first version lets have RuleBody and RuleName (make both required, we can relax it laster). The interesting one is the RuleBody, I agree that we should plan for another scheme release, but I don't see a need to create a Scheme type, we can implement different RuleBody for each scheme, i.e RuleBodyV1, this has the advantage of being more explicit.

Lets start with structuring RuleBody , from the CW docs, see comments inline:

    "LogGroupNames": [
        "API-Gateway-Access-Logs*",
        "Log-group-name2"
    ],
    "LogFormat": "JSON", // Enum
    "Contribution": { 
        "Keys": [
            "$.ip"
        ],
        "ValueOf": "$.requestBytes",
        "Filters": [ // lets make it a type
            {
                "Match": "$.httpMethod",
                "In": [
                    "PUT"
                ]
            }
        ]
    },
    "AggregateOn": "Sum" // Enum

Lets create an Interface for IRuleBodyV1 with a render(). In the next version we can create convenient methods like RuleBodyV1.FromLogGroup() .

WDYTH?

iph commented 3 years ago

Ooo, yeah, much more succinct. I'll try to write up a quick draft on Monday to see if we can start iterating on just the base. I agree that the bells and whistles can come later.

jerry-shao commented 2 years ago

Hi @iph

are you still working on the issue, or it's ok for me to pick up based on your work?