Add synthetics locations data source

[dabcoder]

Attempts to address #217.
related to https://github.com/terraform-providers/terraform-provider-datadog/issues/381

[nmuesch]

Overall looks good, thanks for this. Left a few small notes/questions.

[nmuesch]

Whats this if section for ?

[dabcoder]

In case the list of locations returned by the Go client is nil, we still set a locations attribute but with no values. Similar to https://github.com/terraform-providers/terraform-provider-datadog/blob/master/datadog/data_source_datadog_ip_ranges.go#L113:L118

[nmuesch]

Any use case for returning the rest of the values this API endpoint returns (id, region, etc) ? We can add new schemas here later but changing this locations attribute to include them later will be challenging if we want them.

[dabcoder]

I don't think so, in order to create a test (with this data source location), the "name" is the only attribute used: https://docs.datadoghq.com/api/?lang=bash#create-a-test.
If we go that route we may want to update the attribute to locations_name maybe?
If we want to fetch all attributes from the locations I am not sure how this would work, with a list of maps perhaps (my testing isn't working in that case). EDIT: maybe with something similar to https://github.com/terraform-providers/terraform-provider-github/blob/0a3a420af42e3124b7972ff2e4fb56d949688c4f/github/data_source_github_collaborators.go#L157-L181.

[gzussa]

@btoueg We need your input on this one 😄

[btoueg]

I recommend we keep at least region, display_name and name, e.g.:

[{"region":"Asia Pacific","display_name":"Tokyo (AWS)","name":"aws:ap-northeast-1"}, ...]

How is that data supposed to be used? Can we provide an example in the doc?

I'm afraid an array will be less usable than a map

[dabcoder]

Thanks for the feedback.

My understanding is that it would be defined as a data source such as in the below:

data "synthetics_locations" "example" {}

... in order to fetch all active locations and use them when creating/editing a test using something along the lines of:

resource "datadog_synthetics_test" "test_api" {
  type = "api"
  subtype = "http"
  request = {
    method = "GET"
    url = "https://www.example.org"
  }
  request_headers = {
    Content-Type = "application/json"
    Authentication = "Token: 1234566789"
  }
  assertions = [
    {
      type = "statusCode"
      operator = "is"
      target = "200"
    }
  ]
  locations = data.synthetics_locations.example.location_names
...
}

Instead of hardcoding all active locations one by one.

[dabcoder]

I'm afraid an array will be less usable than a map

According to https://www.terraform.io/docs/providers/datadog/r/synthetics.html#example-usage-synthetics-api-test-, locations needs to be a list, correct?

[btoueg]

Yes it does need to be a list. It all depends how you want to use it, for instance, it could be something like:

resource "datadog_synthetics_test" "test_api" {
  ...
  locations = [
    data.synthetics_locations["Tokyo"],
    data.synthetics_locations["Sidney"],
  ]
...
}

Example above is not syntactically sound but you get the idea

[bkabrda]

So assuming the name (e.g. aws:us-west-2) is unique for all the locations (CC @btoueg - is it guaranteed to be unique?), I think the optimal solution would be to have locations as map:

"aws:us-west-2": {
    "id": 123,
    "display_name":"Oregon (AWS)",
    ... 
}

and then users would be able to set up locations for the datadog_synthetics_test such as:

resource "datadog_synthetics_test" "test_api" {
    locations = data.synthetics_locations.example.locations.keys
}

That seems reasonably easy to use and allows us to carry additional data if/when necessary. Assuming it's not guaranteed that location names are unique, we'll have to go with list of maps and figure out a clever oneliner to access all the names (which, thinking about it, wouldn't make much sense if names weren't unique, so they probably are...) :)

[dabcoder]

name should be unique indeed, AWS regions don't include duplicates.
I've created a map here: https://github.com/terraform-providers/terraform-provider-datadog/pull/309/files#diff-247b9e616dd8b78804cde056db1f776aR23 and filled it with region, display_name and name so far.
Is that the right approach you think? Should we fetch more keys such as the ids, is_active?

[bkabrda]

@dabcoder what I meant was to make the top-level locations attribute not a list, but a map - so that it would become more easily accessible. IOW, the current attribute you use to set locations is locationsList and is a []string - I'm suggesting that you make this a map[string]LocationMap. Does that make sense?

[btoueg]

If there's only one field to keep, it's Name rather than DisplayName

Suggested change

	locationsList = append(locationsList, location.GetDisplayName())
	locationsList = append(locationsList, location.GetName())

[btoueg]

I recommend we keep at least region, display_name and name, e.g.:

[{"region":"Asia Pacific","display_name":"Tokyo (AWS)","name":"aws:ap-northeast-1"}, ...]

How is that data supposed to be used? Can we provide an example in the doc?

I'm afraid an array will be less usable than a map

[zippolyte]

What's this for ?

[therve]

We only set the ID if it's not empty, but we still set the map. I guess it should be outside the condition, let me fix it.

Stale

[jirikuncar]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[therve]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).