aws-ia / cloudformation-dynatrace-resource-providers

Dynatrace CFN Registry resource
Apache License 2.0
6 stars 7 forks source link

Error: Could not map JSON at '' - Synthetic Monitor #21

Open amorrisoon opened 8 months ago

amorrisoon commented 8 months ago

I am trying to deploy a synthetic monitor using CDK L1 constructs.

I have activated the cloudformation extension, provided the correct configuration and token with appropriate permissions on dynatrace but when running cdk deploy I get this error:

Resource handler returned message: "Error: Could not map JSON at '' near line 1 column 63" 
(RequestToken: cf1e799d-ae43-bd26-4aa5-c47b550c5a22, HandlerErrorCode: InvalidRequest)

My cdk config


import { CfnSyntheticMonitor, CfnSyntheticMonitorPropsFrequencyMin } from "@cdk-cloudformation/dynatrace-environment-syntheticmonitor";
import * as cdk from "aws-cdk-lib";
import { Construct } from "constructs";

export class MonitoringStack extends cdk.Stack {
  constructor(scope: Construct, id: string, props?: cdk.StackProps) {
    super(scope, id, props);

    new CfnSyntheticMonitor(this, "MyNewMonitor", {
      name: "MyNewMonitor",
      enabled: false,
      frequencyMin: CfnSyntheticMonitorPropsFrequencyMin.VALUE_5,
    });
  }
}

Generated Template

{
 "Resources": {
  "MyNewMonitor": {
   "Type": "Dynatrace::Environment::SyntheticMonitor",
   "Properties": {
    "FrequencyMin": 5,
    "Enabled": false,
    "Name": "MyNewMonitor"
   },
   "Metadata": {
    "aws:cdk:path": "my-branch/MyNewMonitor"
   }
  },
  "CDKMetadata": {
   "Type": "AWS::CDK::Metadata",
   "Properties": {
    "Analytics":  ...
   },
   "Metadata": {
    "aws:cdk:path": "my-branch/CDKMetadata/Default"
   }
  }
 },
 "Parameters": {
  "BootstrapVersion": {
   "Type": "AWS::SSM::Parameter::Value<String>",
   "Default": "/cdk-bootstrap/.../version",
   "Description": "Version of the CDK Bootstrap resources in this environment, automatically retrieved from SSM Parameter Store. [cdk:skip]"
  }
 },
 "Rules": {
  "CheckBootstrapVersion": {
   "Assertions": [
    {
     "Assert": {
      "Fn::Not": [
       {
        "Fn::Contains": [
         [
          "1",
          "2",
          "3",
          "4",
          "5"
         ],
         {
          "Ref": "BootstrapVersion"
         }
        ]
       }
      ]
     },
     "AssertDescription": "CDK bootstrap stack version 6 required. Please run 'cdk bootstrap' with a recent version of the CDK CLI."
    }
   ]
  }
 }
}

packages

 "devDependencies": {
    "@types/aws-lambda": "^8.10.119",
    "@types/jest": "^29.5.1",
    "@types/node": "20.1.7",
    "aws-cdk": "2.96.2",
    "jest": "^29.5.0",
    "ts-jest": "^29.1.0",
    "ts-node": "^10.9.1",
    "typescript": "~5.0.4"
  },
  "dependencies": {
    "@aws-sdk/client-cloudformation": "^3.425.0",
    "@cdk-cloudformation/dynatrace-environment-syntheticlocation": "1.2.0-alpha.7",
    "@cdk-cloudformation/dynatrace-environment-syntheticmonitor": "1.2.0-alpha.7",
    "aws-cdk-lib": "2.83.1",
    "constructs": "^10.0.0",
    "dotenv": "^16.3.1",
    "source-map-support": "^0.5.21"
  }
}
LucasQChen commented 8 months ago

@amorrisoon the team is having a look at this

LucasQChen commented 8 months ago

@amorrisoon quick update, team is still looking into it and thinking the issue is with the underlying resource (rather than CDK). Investigation is ongoing and will update here with next steps once we have them.

LucasQChen commented 8 months ago

@amorrisoon could you try with more properties to see if you get the same error? It looks like the underlying Dynatrace API requires more than 3 parameters. I see the resource schema says only 3 are required, but I think that may need fixing.

Can you try including the properties that are in this example? https://github.com/aws-ia/cloudformation-dynatrace-resource-providers/blob/eadf23add5a520a2a91f2aaad40b5256435a7efd/Dynatrace-Environment-SyntheticMonitor/README.md#shows-how-to-create-a-synthetic-monitor-in-dynatrace

amorrisoon commented 8 months ago

@LucasQChen I did try an more expansive construct before opening this issue, but not with all those fields in your example.

I will try with the example above and get back to you.

amorrisoon commented 8 months ago

@LucasQChen

So i managed to get this working with following your example, here is the construct i created that worked.


new CfnSyntheticMonitor(this, 'MyNewMonitor', {
      name: 'MyNewMonitor',
      enabled: true,
      frequencyMin: CfnSyntheticMonitorPropsFrequencyMin.VALUE_5,
      type: CfnSyntheticMonitorPropsType.HTTP,
      script: {
        version: '1.0',
        requests: [
          {
            url: 'https://www.mysite.com/',
            method: 'GET',
            description: 'Page test',
            validation: {
              rules: [
                {
                  type: 'httpStatusesList',
                  value: '500, 400',
                  passIfFound: false
                },
                {
                  type: 'httpStatusesList',
                  value: '200, 204, 302',
                  passIfFound: true
                }
              ]
            }
          }
        ]

      },
      anomalyDetection: {
        outageHandling: {
          globalOutage: true,
          globalOutagePolicy: {
            consecutiveRuns: 3,
          },
          localOutage: true,
          localOutagePolicy: {
            affectedLocations: 1,
            consecutiveRuns: 3
          }
        },
        loadingTimeThresholds: {
          enabled: true,
          thresholds: [
            {
              type: LoadingTimeThresholdType.TOTAL,
              valueMs: 100
            }
          ]
        }
      },
      locations: [
        'GEOLOCATION-F3E06A526BE3B4C4'
      ]
    })

Somethings i noticed where that this does not deploy when setting the monitor type to BROWSER I recieved the following error:

"violations":[{"path":"/requests/0","message":"Illegal property \"requests/0\""}]

There is a difference in the required input for Script when deploying a browser monitor, the CfnSyntheticMonitorPropsScript type does not seem to support any of the required inputs, found here for browser monitors.

The documentation does say...


/**
 * The script of a browser (https://dt-url.net/9c103rda) or HTTP monitor.
 *
 * @schema CfnSyntheticMonitorPropsScript
 */

but i am unsure of whow this would work given the current types, could you please provide an example implementation for a browser synthetic monitor

zan-mateusz commented 8 months ago

Hi @amorrisoon,

My name is Mateusz Zan and I am part of the team responsible for the Dynatrace Cloudformation resources. Apologies for the issues that you are facing, but rest assured we will try to provide you with the resolution as soon as possible. I have looked into this in more details and have some conclusions.

First, with regards to the documentation on CfnSyntheticMonitorPropsScript, I appreciate that there is not much details for the that parameter. This is due to the fact that the Script parameter has been set as an object without any specific constraints, to cover different cases for different type of monitors. User can insert any properties and values into that object. Please refer to the documentation shared previously by @LucasQChen to see what properties are accepted by each type of monitors.

I am glad to see that you have managed to get HTTP type monitor to be created successfully with the data above and from what I understand you have also tried to simply change the type to BROWSER, leaving the other data intact. This causes the error you have noted: "Illegal property \"requests/0\"" - since requests is only valid for the HTTP type of scripts. For BROWSER scripts we would need events and type instead (again please refer to the documentation shared above to see more details on this). See an example of a valid script definition for browser monitor below:

"Script": {
    "Version": "1.0",
    "Events": [
      {
        "Description": "Loading of example.com",
        "Type": "navigate",
        "Url": "http://example.com/",
        "Wait": {
          "WaitFor": "page_complete"
        }
      }
    ],
    "Type": "availability"
  }

That helps with the error that you are facing. Unfortunately, we have identified another potential issue, as the BROWSER type monitor seems to require an additional parameter - keyPerformanceMetrics, which is currently not part of the schema. Documentation for the api is somehow unhelpful as it doesn't describe that parameter, it only presents it in the example request. With some minor modification to the resource on my end, I was able to successfully create a browser monitor with the following input data:

{
  "Type": "BROWSER",
  "FrequencyMin": 5,
  "Enabled": true,
  "Name": "TestMonitor1",
  "Locations": [
    "GEOLOCATION-B1B096907F0E9A8C"
  ],
  "Script": {
    "Version": "1.0",
    "Events": [
      {
        "Description": "Loading of example.com",
        "Type": "navigate",
        "Url": "http://example.com",
        "Wait": {
          "WaitFor": "page_complete"
        }
      }
    ],
    "Type": "availability"
  },
  "Tags": [
    "example"
  ],
  "ManuallyAssignedApps": [],
  "KeyPerformanceMetrics": {
    "LoadActionKpm": "VISUALLY_COMPLETE",
    "XhrActionKpm": "VISUALLY_COMPLETE"
  }
}

Note that the resource needs to be updated to support the missing parameter in order to work on your end. We are looking to fix the issue as soon as possible.

If you have any questions in the meantime, please do not hesitate to let me know. Thanks, Mateusz Zan

amorrisoon commented 8 months ago

Hi @zan-mateusz

Thanks for getting back to me, while i understand the underlying resource provided by dynatrace allows the updated script configuration.

What I was trying to explain that is when implementing this template using CDK with typescript the actual typescript types provided by the cdk package do not allow me to place the events, type and any other fields required for the browser monitor into the script field as they are do not seem to be support by this type.

Trying to place them in there will cause errors when trying to run cdk synth.

I was wondering if there was anyway of getting around this or wether the actual types need to be updated on that pacakge?

Here is an example error I get when trying to implement a browser monitor script:

image

zan-mateusz commented 8 months ago

Hi @amorrisoon, apologies for a late reply on this issue.

Also, apologies for the confusion in my last message. Due to some technical error, the codebase that I was using was not up to date and therefore I didn't see the structure of the scripts object within the schema properly. Fortunately, after synching up properly, I can see that the script object indeed has the requests property, explaining the error you can see in CDK.

Please rest assured that this will be fixed soon, by updating the schema to support the properties missing for the Browser type monitors. This has already been done internally and verified that this works in creation and management of the relevant resources, however the Dynatrace API at times throw errors (503 and 404) on these resources, hence we are currently working on a fix for this issue in order to ensure that there will be no functionality issues once released.

This should be completed in a timely manner and I will inform you here in the issue once this has been updated officially. If you have any questions in the meantime, please do not hesitate to reach out and I will be happy to help! Thanks, Mateusz

amorrisoon commented 8 months ago

@zan-mateusz No worries thanks for getting back to me, good to see my understanding was correct 😆.

amorrisoon commented 7 months ago

@zan-mateusz Hey I see your pull request, i was wondering if there was any updates on when this will get merged?

LucasQChen commented 7 months ago

Hello @amorrisoon I can help answer that. We have a few other items in our publishing backlog ahead of this Dynatrace update. With that and given the upcoming holidays, I think a realistic estimate is after the new year in January. If our team is able to get it done any sooner, I will update you here.

amorrisoon commented 4 months ago

Hey @LucasQChen @zan-mateusz just following up on this as i see the PR is still open, any updates and when we can expect this fix?