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]: Support managing Monte Carlo monitors #52

Open
4 tasks done
mubarak-j opened this issue Oct 27, 2023 · 4 comments
Open
4 tasks done

[Feature]: Support managing Monte Carlo monitors #52

mubarak-j opened this issue Oct 27, 2023 · 4 comments
Assignees
Labels
feature New feature or request triage Not yet considered

Comments

@mubarak-j
Copy link

Contact Details

[email protected]

Feature Request Details

Thank you for all the great work on this valuable provider in such a short time, much appreciated! 🙌🏼

If this is something aligned with the provider roadmap, I'd like to request adding the support for creating, managing, and importing Monte Carlo monitors.

I understand that MC monitors vary in type and API fields and this might be a complex effort, please feel free to break up this effort into multiple issues and/or TF resources. Thank you!

Acceptance Criteria

Importance of the feature

must have

Checklist

  • I agree to follow this project's Code of Conduct
  • I have reviewed the documentation and other relevant resources.
  • I have searched for similar issues in the repository.
  • I have provided all the requested information.
@mubarak-j mubarak-j added feature New feature or request triage Not yet considered labels Oct 27, 2023
@ndopj
Copy link
Contributor

ndopj commented Oct 28, 2023

Hello @mubarak-j 👋

Right now I am working on Monte Carlo's authorization groups. This effort will result into two new resources montecarlo_iam_group and montecarlo_iam_member (non-authoritative assignment to the group). Those will be preferably part of the 0.3.0 release, with montecarlo_iam_group already being merged to the master.

Once this implementation is finished I can start working on this issue. Originally I did not plan to implement monitors functionality in this provider, since alternative solution is using Monitors as Code architecture (e.g. deploying monitors from git repositories). However, if you find this functionality really useful I'll definitely look into its implementation (if its possible via API).

In the meantime, could you please describe how the solution should work ?

  • Should there be a single Terraform resource loading monitor specification from .yml file similarly to how Monitors as Code architecture works ? For example:
resource "montecarlo_monitor" "example" {
  monitor          = file("${path.module}/mc_monitors/my_monitor.yml")
  namespace        = "namespace"
  default_resource = "bigquery-warehouse-main"
}
  • Or would you rather like to have specific Terraform resources for each kind of monitor (with this option being harder to implement and manage overall).
resource "montecarlo_monitor_field_health" "example" {
  table            = "project:dataset.table_name"
  timestamp_field  = "created"
  namespace        = "namespace"
  default_resource = "bigquery-warehouse-main"
}

resource "montecarlo_monitor_dimension_tracking" "example" {
  table            = "project:dataset.table_name"
  timestamp_field  = "created"
  field            = "order_status"
  namespace        = "namespace"
  default_resource = "bigquery-warehouse-main"
}

Thank you very much for opening this issue and contributing to this provider ! I really appreciate issues like this because it makes this provider more generally applicable. If you know about any other functionalities, that you would like this provider to implement, feel free to open another issue !

Thanks in advance for your response and your time !

@ndopj ndopj added this to the 0.4.0 milestone Oct 28, 2023
@mubarak-j
Copy link
Author

Thank you for considering this feature. I think the first option will work for us and probably allow this provider more flexibility with MC API changes. I imagine we could use the yaml monitor file in an internal module and iterate over various fields/variables using TF templatefile function to spin up different monitors.

@ndopj ndopj added wip Work in progress and removed triage Not yet considered labels Nov 28, 2023
@ndopj
Copy link
Contributor

ndopj commented Dec 1, 2023

@mubarak-j 👋 Hope you are doing well.

I finally found some time for a simple refactoring in this repository (restructuring packages and reworking the acceptance tests to work against a real Monte Carlo instance), and I've started working on this issue.

So far, I can verify that the requested functionality can indeed be achieved via this endpoint:
https://apidocs.getmontecarlo.com/#mutation-createOrUpdateMonteCarloConfigTemplate

However, it seems a lot of unusual approaches (for a terraform provider) will have to be used, e.g., implementing a read operation for this use case. Therefore, it might require a bit more effort than other resources.

I'll keep you updated here.

@ndopj ndopj linked a pull request Dec 1, 2023 that will close this issue
@ndopj ndopj added triage Not yet considered and removed wip Work in progress labels Dec 6, 2023
@ndopj
Copy link
Contributor

ndopj commented Dec 6, 2023

So, I've tried implementing this through multiple approaches, but unfortunately, it seems that the option we agreed upon will not work with Terraform. However, I still believe that the monitors could be implemented using the second proposed solution. With the experience gained from previous attempts, I genuinely think the second option is ultimately simpler to implement.

Before summarizing my experience thus far, I would like to once again highlight and emphasize that the Monte Carlo provided solution, Monitors as a Code (MaaC), should remain the preferred method for addressing this issue.

Approach by loading monitor specification from .yml file

resource "montecarlo_monitor" "example" {
  default_warehouse = "TestAccWarehouseDataSource"
  configuration     = file("./monitors/monitor.yaml")
}

Namespace

Each Terraform resource can run in parallel with other resources and, for this reason, should be idempotent. Consequently, the resource example above needed to generate a unique namespace for each monitor .yml file specification. The namespace not only groups monitors based on the deployment process but also serves as an indicator for MaaC operations to identify which monitors are subject to specific operations. It's important to note that included monitor .yml file specifications could not share the same namespace across multiple resources, as MaaC operations executed by a single resource were not aware of monitor definitions in other resources.

  • While this issue did not hinder a successful implementation, the approach mentioned would not allow for the import of already existing monitors into the Terraform resources.

State issues

Storing monitor configuration inside a file used as input for the aforementioned resource results in non-standard Terraform output, as shown in the following example. This behavior could potentially impact general Terraform usage.

  # montecarlo_monitor.example will be updated in-place
  ~ resource "montecarlo_monitor" "example" {
      ~ configuration     = <<-EOT
            montecarlo:
          -     dimension_tracking:
          -         - aggregation_time_interval: hour
          -           description: Dimension Tracking for product_name in data-playground-8bb9fc23:terraform_provider_montecarlo.device
          -           field: product_name
          -           lookback_days: 1
          -           notify_rule_run_failure: true
          -           resource: TestAccWarehouseDataSource
          -           schedule:
          -             interval_minutes: 720
          -             start_time: "2023-12-04T12:53:06.823000+00:00"
          -             timezone: UTC
          -             type: fixed
          -           table: data-playground-8bb9fc23:terraform_provider_montecarlo.device
          -           use_partition_clause: false
          +     dimension_tracking:
          +         - resource: TestAccWarehouseDataSource
          +         table: data-playground-8bb9fc23:terraform_provider_montecarlo.device
          +         lookback_days: 1
          +         aggregation_time_interval: hour
          +         schedule:
          +             type: fixed
          +             interval_minutes: 720
          +             start_time: '2023-12-04T12:53:06.823000+00:00'
          +             timezone: UTC
          +            description: Dimension Tracking for product_name in data-playground-8bb9fc23:terraform_provider_montecarlo.device
          +         notify_rule_run_failure: true
          +         field: product_name
          +         use_partition_clause: false
        EOT
        # (2 unchanged attributes hidden)
    }

Passing monitor configurations through the configuration attribute of the resource to the logic applying these configuration files was the only viable solution. Since each resource attribute is stored in the Terraform state, to accurately display and execute modifications to the monitors represented by such a resource, this attribute would need to be updated with values that capture these operations. Two approaches were attempted to implement this solution."

  • detecting the diff in remote state and applying it to the .yml configuration. There are lot of complications with this approach mainly related to how diff is obtained from the API and afterwards applied to the configuration attribute. API is able to capture capture current state of the monitors (as well as currently applied configuration file), but the elements are re-ordered and since the configuration attribute input value does not have to necessarily carry any unique identifier of attached monitors it can grow in complexity. Additionally, different monitor types have different 'diff' behaviours, which is knowledge not shared outside of the Monte Carlo.

  • By using additional attributes to capture the current state of the monitors represented by this resource, this solution could theoretically work. However, it leads to inconsistent Terraform behavior. The resource would fill in the current state of the monitors into a separate read-only attribute. This separate attribute could then be used to detect drift changes. Yet, updates to the input configuration attribute would not be reflected during the same Terraform plan action in this additional attribute, resulting in inconsistent behavior. This could also lead to a scenario where, if there were changes in the remote and users attempted to update the local configuration file accordingly, Terraform would still report changes in the remote, despite the local configuration file matching the remote state of the monitors.

    {
      "mode": "managed",
      "type": "montecarlo_monitor",
      "name": "example",
      "provider": "provider[\"registry.terraform.io/kiwicom/montecarlo\"]",
      "instances": [
        {
          "schema_version": 0,
          "attributes": {
            "configuration": "---\nmontecarlo:\n  json_schema:\n  - resource: TestAccWarehouseDataSource\n    table: data-playground-8bb9fc23:terraform_provider_montecarlo.device\n    lookback_days: 1\n    aggregation_time_interval: day\n    schedule:\n      type: fixed\n      interval_minutes: 720\n      start_time: '2023-12-04T12:01:18.703000+00:00'\n      timezone: UTC\n    description: JSON Schema monitor for quantity in device\n    notify_rule_run_failure: true\n    field: quantity\n  dimension_tracking:\n  - resource: TestAccWarehouseDataSource\n    table: data-playground-8bb9fc23:terraform_provider_montecarlo.device\n    lookback_days: 1\n    aggregation_time_interval: hour\n    schedule:\n      type: fixed\n      interval_minutes: 720\n      start_time: '2023-12-04T12:53:06.823000+00:00'\n      timezone: UTC\n    description: Dimension Tracking for product_name in data-playground-8bb9fc23:terraform_provider_montecarlo.device\n    notify_rule_run_failure: true\n    field: product_name\n    use_partition_clause: false\n  - resource: TestAccWarehouseDataSource\n    name: lol\n    table: data-playground-8bb9fc23:terraform_provider_montecarlo.device\n    lookback_days: 1\n    aggregation_time_interval: hour\n    schedule:\n      type: fixed\n      interval_minutes: 720\n      start_time: '2023-12-04T12:53:06.823000+00:00'\n      timezone: UTC\n    description: Dimension Tracking for product_name in data-playground-8bb9fc23:terraform_provider_montecarlo.device\n    notify_rule_run_failure: true\n    field: product_name\n    use_partition_clause: false",
            "default_warehouse": "TestAccWarehouseDataSource",
            "resources": {
              "0fe1f21c-dff5-4b74-9256-e5a675569e79": "monitor|type=categories|table=data-playground-8bb9fc23:terraform_provider_montecarlo.device|timestamp_field=\u003c\u003cNULL\u003e\u003e|where_condition=\u003c\u003cNULL\u003e\u003e|field=product_name",
              "b556afac-6762-4bb7-9375-013063f024a4": "monitor|type=json_schema|table=data-playground-8bb9fc23:terraform_provider_montecarlo.device|timestamp_field=\u003c\u003cNULL\u003e\u003e|where_condition=\u003c\u003cNULL\u003e\u003e|field=quantity",
              "ba92463f-b8b3-4a2b-ae55-37a61e52b5d9": "lol"
            },
            "uuid": "deb666ec-c0ae-49ce-bcb8-f8ed65d27c09"
          },
          "sensitive_attributes": []
        }
      ]
    }

Resolution

Overall, it appears that integrating Terraform with an additional Infrastructure as a Code tool like Monte Carlo MaaC does not yield smooth integration. While the aforementioned approach could be implemented with sufficient motivation, it would significantly increase the complexity of this provider and introduce behavior that is not standard for Terraform providers."

@mubarak-j 👋 I will proceed with the implementation using the specific monitor resources (option 2 that we discussed), likely beginning with the Custom SQL Rule monitor. For more effective management, new issues should be created for each monitor 'type.' Therefore, if there is any monitor type you would like to see supported promptly, please feel free to open a new feature request for that specific monitor type and link to this issue within it.

@ndopj ndopj removed this from the 0.4.0 milestone Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request triage Not yet considered
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants