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

azurerm_api_management_diagnostic add additional options, fix log_client_ip #10325

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ func resourceApiManagementApiDiagnosticCreateUpdate(d *schema.ResourceData, meta
}
}

if logClientIP, ok := d.GetOk("log_client_ip"); ok {
logClientIP, _ := d.GetOk("log_client_ip")
if logClientIP != nil {
parameters.LogClientIP = utils.Bool(logClientIP.(bool))
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @slynickel, many thanks for the PR.

Because this attribute is optional+computed, it would be best to use GetOkExists() which tells you if the attribute is configured, but bypasses the zero value check so you can detect an explicit false value.

Suggested change
logClientIP, _ := d.GetOk("log_client_ip")
if logClientIP != nil {
parameters.LogClientIP = utils.Bool(logClientIP.(bool))
}
if logClientIP, exists := d.GetOkExists("log_client_ip"); exists {
parameters.LogClientIP = utils.Bool(logClientIP.(bool))
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@manicminer Isn't that function being deprecated though? ResourceData.GetOkayExist()

GetOkExists can check if TypeBool attributes that are Optional with no Default value have been set.

Deprecated: usage is discouraged due to undefined behaviors and may be removed in a future version of the SDK

My light review of the terraform-plugin-sdk is that some sections are pretty old, though the deprecation warning is within the last year.

Copy link
Contributor

@manicminer manicminer Jan 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slynickel I checked with the team and although it's marked as deprecated in the SDK, in a context like this (specifically optional+computed booleans) there's little alternative and it's ok to use it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

Make sense, thanks for context.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@manicminer I'm assuming the failed golint check as a result of GetOkExist can be ignored?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@slynickel we can add a nolint comment to suppress that, i've pushed that whilst I run the acceptance tests


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,55 @@ func resourceApiManagementDiagnostic() *schema.Resource {
Optional: true,
Deprecated: "this property has been removed from the API and will be removed in version 3.0 of the provider",
},

"sampling_percentage": {
Type: schema.TypeFloat,
Optional: true,
Computed: true,
ValidateFunc: validation.FloatBetween(0.0, 100.0),
},

"always_log_errors": {
Type: schema.TypeBool,
Optional: true,
Computed: true,
},

"verbosity": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ValidateFunc: validation.StringInSlice([]string{
string(apimanagement.Verbose),
string(apimanagement.Information),
string(apimanagement.Error),
}, false),
},

"log_client_ip": {
Type: schema.TypeBool,
Optional: true,
Computed: true,
},

"http_correlation_protocol": {
Type: schema.TypeString,
Optional: true,
Computed: true,
ValidateFunc: validation.StringInSlice([]string{
string(apimanagement.HTTPCorrelationProtocolNone),
string(apimanagement.HTTPCorrelationProtocolLegacy),
string(apimanagement.HTTPCorrelationProtocolW3C),
}, false),
},

"frontend_request": resourceApiManagementApiDiagnosticAdditionalContentSchema(),

"frontend_response": resourceApiManagementApiDiagnosticAdditionalContentSchema(),

"backend_request": resourceApiManagementApiDiagnosticAdditionalContentSchema(),

"backend_response": resourceApiManagementApiDiagnosticAdditionalContentSchema(),
},
}
}
Expand Down Expand Up @@ -95,6 +144,70 @@ func resourceApiManagementDiagnosticCreateUpdate(d *schema.ResourceData, meta in
},
}

if samplingPercentage, ok := d.GetOk("sampling_percentage"); ok {
parameters.Sampling = &apimanagement.SamplingSettings{
SamplingType: apimanagement.Fixed,
Percentage: utils.Float(samplingPercentage.(float64)),
}
} else {
parameters.Sampling = nil
}

if alwaysLogErrors, ok := d.GetOk("always_log_errors"); ok && alwaysLogErrors.(bool) {
parameters.AlwaysLog = apimanagement.AllErrors
}

if verbosity, ok := d.GetOk("verbosity"); ok {
switch verbosity.(string) {
case string(apimanagement.Verbose):
parameters.Verbosity = apimanagement.Verbose
case string(apimanagement.Information):
parameters.Verbosity = apimanagement.Information
case string(apimanagement.Error):
parameters.Verbosity = apimanagement.Error
}
slynickel marked this conversation as resolved.
Show resolved Hide resolved
}

logClientIP, _ := d.GetOk("log_client_ip")
if logClientIP != nil {
parameters.LogClientIP = utils.Bool(logClientIP.(bool))
}

if httpCorrelationProtocol, ok := d.GetOk("http_correlation_protocol"); ok {
switch httpCorrelationProtocol.(string) {
case string(apimanagement.HTTPCorrelationProtocolNone):
parameters.HTTPCorrelationProtocol = apimanagement.HTTPCorrelationProtocolNone
case string(apimanagement.HTTPCorrelationProtocolLegacy):
parameters.HTTPCorrelationProtocol = apimanagement.HTTPCorrelationProtocolLegacy
case string(apimanagement.HTTPCorrelationProtocolW3C):
parameters.HTTPCorrelationProtocol = apimanagement.HTTPCorrelationProtocolW3C
}
slynickel marked this conversation as resolved.
Show resolved Hide resolved
}

frontendRequest, frontendRequestSet := d.GetOk("frontend_request")
frontendResponse, frontendResponseSet := d.GetOk("frontend_response")
if frontendRequestSet || frontendResponseSet {
parameters.Frontend = &apimanagement.PipelineDiagnosticSettings{}
if frontendRequestSet {
parameters.Frontend.Request = expandApiManagementApiDiagnosticHTTPMessageDiagnostic(frontendRequest.([]interface{}))
}
if frontendResponseSet {
parameters.Frontend.Response = expandApiManagementApiDiagnosticHTTPMessageDiagnostic(frontendResponse.([]interface{}))
}
}

backendRequest, backendRequestSet := d.GetOk("backend_request")
backendResponse, backendResponseSet := d.GetOk("backend_response")
if backendRequestSet || backendResponseSet {
parameters.Backend = &apimanagement.PipelineDiagnosticSettings{}
if backendRequestSet {
parameters.Backend.Request = expandApiManagementApiDiagnosticHTTPMessageDiagnostic(backendRequest.([]interface{}))
}
if backendResponseSet {
parameters.Backend.Response = expandApiManagementApiDiagnosticHTTPMessageDiagnostic(backendResponse.([]interface{}))
}
}

if _, err := client.CreateOrUpdate(ctx, resourceGroup, serviceName, diagnosticId, parameters, ""); err != nil {
return fmt.Errorf("creating or updating Diagnostic %q (Resource Group %q / API Management Service %q): %+v", diagnosticId, resourceGroup, serviceName, err)
}
Expand Down Expand Up @@ -136,6 +249,29 @@ func resourceApiManagementDiagnosticRead(d *schema.ResourceData, meta interface{
d.Set("resource_group_name", diagnosticId.ResourceGroup)
d.Set("api_management_name", diagnosticId.ServiceName)
d.Set("api_management_logger_id", resp.LoggerID)
if props := resp.DiagnosticContractProperties; props != nil {
if props.Sampling != nil && props.Sampling.Percentage != nil {
d.Set("sampling_percentage", props.Sampling.Percentage)
}
d.Set("always_log_errors", props.AlwaysLog == apimanagement.AllErrors)
d.Set("verbosity", props.Verbosity)
d.Set("log_client_ip", props.LogClientIP)
d.Set("http_correlation_protocol", props.HTTPCorrelationProtocol)
if frontend := props.Frontend; frontend != nil {
d.Set("frontend_request", flattenApiManagementApiDiagnosticHTTPMessageDiagnostic(frontend.Request))
d.Set("frontend_response", flattenApiManagementApiDiagnosticHTTPMessageDiagnostic(frontend.Response))
} else {
d.Set("frontend_request", nil)
d.Set("frontend_response", nil)
}
if backend := props.Backend; backend != nil {
d.Set("backend_request", flattenApiManagementApiDiagnosticHTTPMessageDiagnostic(backend.Request))
d.Set("backend_response", flattenApiManagementApiDiagnosticHTTPMessageDiagnostic(backend.Response))
} else {
d.Set("backend_request", nil)
d.Set("backend_response", nil)
}
}

return nil
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,21 @@ func TestAccApiManagementDiagnostic_requiresImport(t *testing.T) {
})
}

func TestAccApiManagementDiagnostic_complete(t *testing.T) {
data := acceptance.BuildTestData(t, "azurerm_api_management_diagnostic", "test")
r := ApiManagementDiagnosticResource{}

data.ResourceTest(t, r, []resource.TestStep{
{
Config: r.complete(data),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
),
},
data.ImportStep(),
})
}

func (t ApiManagementDiagnosticResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) {
diagnosticId, err := parse.DiagnosticID(state.ID)
if err != nil {
Expand Down Expand Up @@ -177,3 +192,41 @@ resource "azurerm_api_management_diagnostic" "import" {
}
`, r.basic(data))
}

func (r ApiManagementDiagnosticResource) complete(data acceptance.TestData) string {
return fmt.Sprintf(`
%s

resource "azurerm_api_management_diagnostic" "test" {
identifier = "applicationinsights"
resource_group_name = azurerm_resource_group.test.name
api_management_name = azurerm_api_management.test.name
api_management_logger_id = azurerm_api_management_logger.test.id
sampling_percentage = 11.1
always_log_errors = false
log_client_ip = false
http_correlation_protocol = "Legacy"
verbosity = "error"

frontend_request {
body_bytes = 100
headers_to_log = ["Accept"]
}

frontend_response {
body_bytes = 1000
headers_to_log = ["Content-Length"]
}

backend_request {
body_bytes = 1
headers_to_log = ["Host", "Content-Encoding"]
}

backend_response {
body_bytes = 10
headers_to_log = ["Content-Type"]
}
}
`, r.template(data))
}
69 changes: 69 additions & 0 deletions website/docs/r/api_management_diagnostic.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,48 @@ resource "azurerm_api_management_diagnostic" "example" {
resource_group_name = azurerm_resource_group.example.name
api_management_name = azurerm_api_management.example.name
api_management_logger_id = azurerm_api_management_logger.example.id

sampling_percentage = 5.0
always_log_errors = true
log_client_ip = true
verbosity = "Verbose"
http_correlation_protocol = "W3C"

frontend_request {
body_bytes = 32
headers_to_log = [
"content-type",
"accept",
"origin",
]
}

frontend_response {
body_bytes = 32
headers_to_log = [
"content-type",
"content-length",
"origin",
]
}

backend_request {
body_bytes = 32
headers_to_log = [
"content-type",
"accept",
"origin",
]
}

backend_response {
body_bytes = 32
headers_to_log = [
"content-type",
"content-length",
"origin",
]
}
}
```

Expand All @@ -69,6 +111,33 @@ The following arguments are supported:

---


* `always_log_errors` - (Optional) Always log errors. Send telemetry if there is an erroneous condition, regardless of sampling settings.

* `backend_request` - (Optional) A `backend_request` block as defined below.

* `backend_response` - (Optional) A `backend_response` block as defined below.

* `frontend_request` - (Optional) A `frontend_request` block as defined below.

* `frontend_response` - (Optional) A `frontend_response` block as defined below.

* `http_correlation_protocol` - (Optional) The HTTP Correlation Protocol to use. Possible values are `None`, `Legacy` or `W3C`.

* `log_client_ip` - (Optional) Log client IP address.

* `sampling_percentage` - (Optional) Sampling (%). For high traffic APIs, please read this [documentation](https://docs.microsoft.com/azure/api-management/api-management-howto-app-insights#performance-implications-and-log-sampling) to understand performance implications and log sampling. Valid values are between `0.0` and `100.0`.

* `verbosity` - (Optional) Logging verbosity. Possible values are `verbose`, `information` or `error`.

---

A `backend_request`, `backend_response`, `frontend_request` or `frontend_response` block supports the following:

* `body_bytes` - (Optional) Number of payload bytes to log (up to 8192).

* `headers_to_log` - (Optional) Specifies a list of headers to log.

## Attributes Reference

In addition to all arguments above, the following attributes are exported:
Expand Down