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

Floating point rounding issues in 0.15.x #335

Closed
purajit opened this issue Jul 24, 2023 · 3 comments · Fixed by #337
Closed

Floating point rounding issues in 0.15.x #335

purajit opened this issue Jul 24, 2023 · 3 comments · Fixed by #337
Labels

Comments

@purajit
Copy link

purajit commented Jul 24, 2023

Versions
0.15.0, 0.15.1

Steps to reproduce

  1. Add a new honeycombio_trigger. I used
    terraform {
      required_providers {
        honeycombio = {
          source  = "honeycombio/honeycombio"
          version = "= 0.15.0"
        }
      }
    }
    
    provider "honeycombio" {
      api_key = "testing"
    }
    
    resource "honeycombio_trigger" "fp_test" {
      name = "testing"
    
      dataset  = "testing"
      query_id = "testing"
    
      threshold {
        op    = ">"
        value = 1 - 0.99
      }
    }
  2. Run terraform plan
  3. Set provider version to 0.14.0
  4. Run terraform init -upgrade && terraform plan

With 0.15.0 (and 0.15.1), I get the error

│ Error: Provider produced invalid plan
│
│ Provider "registry.terraform.io/honeycombio/honeycombio" planned an invalid value for
│ honeycombio_trigger.fp_test.threshold[0].value: planned value cty.NumberFloatVal(0.01) does not match config value
│ cty.MustParseNumberVal("0.010000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000003").

But with 0.14.0, it generates the expected plan

  # honeycombio_trigger.fp_test will be created
  + resource "honeycombio_trigger" "fp_test" {
      + alert_type = "on_change"
      + dataset    = "testing"
      + frequency  = (known after apply)
      + id         = (known after apply)
      + name       = "testing"
      + query_id   = "testing"

      + threshold {
          + op    = ">"
          + value = 0.01
        }
    }

Additional context

In short, the Honeycomb provider is no longer handling floating point rounding the way it used to. Terraform itself produces floating point inaccuracies for 1 - x, where 0.783 <= x < 1.0, at least on my architecture:

> 1 - 0.782
0.218
> 1 - 0.783
0.21700000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001
> 1 - 0.995
0.004999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999999964

Previously, the provider handled this gracefully, correctly rounding these decimals - but no longer.

This is a pattern we use heavily because we keep a central directory of services, SLOs, etc, and the triggers are generated based on them - so if we have an SLO of 0.995 defined on a service, we generate a failure trigger with threshold 1 - 0.995.

@purajit
Copy link
Author

purajit commented Jul 27, 2023

@jharley this leaves is stuck between a rock and a hard place, since 0.14.0 has the issue with recipients, and this 0.15.0 has this issue. Before we try to make significant workarounds, or look into the issue ourselves, do you have an idea of when this can be triaged and worked on?

Thought about looking at this again, but right now the provider throws this:

Stack trace from the terraform-provider-honeycombio_v0.15.1 plugin:

panic: runtime error: index out of range [-1]

goroutine 4812 [running]:
github.com/honeycombio/terraform-provider-honeycombio/internal/provider.(*triggerResource).Read(0x14000814000, {0x100e18580, 0x1400078b5c0}, {{{{0x100e1d0c8, 0x1400080ef90}, {0x100d1ff20, 0x14000c65cb0}}, {0x100e1e868, 0x140003c0820}}, 0x14000814050, ...}, ...)
	github.com/honeycombio/terraform-provider-honeycombio/internal/provider/trigger_resource.go:397 +0x764
github.com/hashicorp/terraform-plugin-framework/internal/fwserver.(*Server).ReadResource(0x140001d5e40, {0x100e18580, 0x1400078b5c0}, 0x1400078b6e0, 0x1400051b3c0)
	github.com/hashicorp/terraform-plugin-framework@v1.2.0/internal/fwserver/server_readresource.go:97 +0x4a4
github.com/hashicorp/terraform-plugin-framework/internal/proto5server.(*Server).ReadResource(0x140001d5e40, {0x100e18580?, 0x1400078b410?}, 0x14000b102c0)
	github.com/hashicorp/terraform-plugin-framework@v1.2.0/internal/proto5server/server_readresource.go:53 +0x1a4
github.com/hashicorp/terraform-plugin-mux/tf5muxserver.muxServer.ReadResource({0x14000349ad0, 0x14000349b30, {0x140001b1b80, 0x2, 0x2}, {0x0, 0x0, 0x0}, {0x0, 0x0, ...}, ...}, ...)
	github.com/hashicorp/terraform-plugin-mux@v0.10.0/tf5muxserver/mux_server_ReadResource.go:26 +0xdc
github.com/hashicorp/terraform-plugin-go/tfprotov5/tf5server.(*server).ReadResource(0x140002c74a0, {0x100e18580?, 0x1400078a240?}, 0x14000624d20)
	github.com/hashicorp/terraform-plugin-go@v0.15.0/tfprotov5/tf5server/server.go:748 +0x3e8
github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5._Provider_ReadResource_Handler({0x100ddfa00?, 0x140002c74a0}, {0x100e18580, 0x1400078a240}, 0x140003d20e0, 0x0)
	github.com/hashicorp/terraform-plugin-go@v0.15.0/tfprotov5/internal/tfplugin5/tfplugin5_grpc.pb.go:383 +0x170
google.golang.org/grpc.(*Server).processUnaryRPC(0x140002c21e0, {0x100e1d4e0, 0x140004c2680}, 0x140003899e0, 0x140004b45d0, 0x101318970, 0x0)
	google.golang.org/grpc@v1.54.0/server.go:1345 +0xc64
google.golang.org/grpc.(*Server).handleStream(0x140002c21e0, {0x100e1d4e0, 0x140004c2680}, 0x140003899e0, 0x0)
	google.golang.org/grpc@v1.54.0/server.go:1722 +0x82c
google.golang.org/grpc.(*Server).serveStreams.func1.2()
	google.golang.org/grpc@v1.54.0/server.go:966 +0x84
created by google.golang.org/grpc.(*Server).serveStreams.func1
	google.golang.org/grpc@v1.54.0/server.go:964 +0x290

Error: The terraform-provider-honeycombio_v0.15.1 plugin crashed!

@jharley jharley added the bug label Jul 31, 2023
@jharley
Copy link
Collaborator

jharley commented Jul 31, 2023

@purajit I have just confirmed I can reproduce this bug, thanks for reporting it.

This appears to be a difference in how the new Terraform Plugin Framework parses things (0.15 of the provider is a migration of the honeycombio_trigger resource from the Plugin SDKv2 to the new Plugin Framework). I'll see what we can do about a quick fix here 👍🏻

As for the panic, that is another bug that we are aware of but I've yet to be given a reproducible case to trigger it. If you have such a thing I'd very much appreciate you sharing it so we can get that one squashed as well!

@jharley
Copy link
Collaborator

jharley commented Jul 31, 2023

@purajit I've opened hashicorp/terraform-plugin-framework#815 looking for guidance on how to handle this

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

Successfully merging a pull request may close this issue.

2 participants