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

[BUG] Publisher: API Diagnostic are not getting overridden based on configuration.<environment>.yaml #627

Closed
jaliyaudagedara opened this issue Aug 12, 2024 · 20 comments

Comments

@jaliyaudagedara
Copy link

jaliyaudagedara commented Aug 12, 2024

Release version

APIOps Toolkit for Azure APIM v6.0.1

Describe the bug

In the publisher, we can override environment specific values using configuration.<environment>.yaml file.

Supported Scenarios specifies, API Diagnostic (Provides operations for managing Diagnostic settings for the logger in an API) is supported.

And in the sample configuration.prod.yaml, we have this:

apis:
  - name: demo-conference-api
    diagnostics:
      - name: applicationinsights
        properties:
          verbosity: Error
          loggerId: "/subscriptions/[subscription Guid goes here]/resourceGroups/rg-apim-lab-prod/providers/Microsoft.ApiManagement/service/apim-prod-wk-05102022/loggers/[application insights name you chose for the destination environment]"

However it doesn't seem to be working.

In Azure Portal when we enable Diagnostic Logs for an API, it does the following.

curl 'https://management.azure.com/batch?api-version=2016-07-01' \
  --data-raw $'{  
    "requests": [
        {
            "httpMethod": "PUT",
            "relativeUrl": "/subscriptions/<subscriptionId>/resourceGroups/<resourceGroupName>/providers/Microsoft.ApiManagement/service/<apimName>/apis/<apiName>/diagnostics/<diagnosticName>?api-version=2022-09-01-preview",
            "content": {
                "id": "",
                "name": "<diagnosticName>",
                "properties": {
                    "loggerId": "/subscriptions/<subscriptionId>/resourceGroups/<resourceGroupName>/providers/Microsoft.ApiManagement/service/<apimName>/loggers/<loggerName>",
                    "verbosity": "information",
                    // Omitted for brevity
                }
            }
        }
    ]
  }'

I inspected the HTTP calls the publisher is making, and I am not seeing any HTTP call to .../apis/<apiName>/diagnostics/*.
Api Diagnostic - Create Or Update

Expected behavior

The publisher should honor and override API diagnostics as specified in the following format.

apis:
  - name: demo-conference-api
    diagnostics:
      - name: applicationinsights
        properties:
          verbosity: Error
          loggerId: "/subscriptions/[subscription Guid goes here]/resourceGroups/rg-apim-lab-prod/providers/Microsoft.ApiManagement/service/apim-prod-wk-05102022/loggers/[application insights name you chose for the destination environment]"

Actual behavior

The publisher doesn't override API diagnostics.

Reproduction Steps

  1. From the source APIM, create a test HTTP API, and enable Diagnostics Logs
  2. Create a configuration.<environment>.yaml for the target APIM. Update apis.diagnostics with environment specific values
  3. Run publisher and ensure in the target APIM, Diagnostics Log are enabled as per the configuration.<environment>.yaml
Copy link

  Thank you for opening this issue! Please be patient while we will look into it and get back to you as this is an open source project. In the meantime make sure you take a look at the [closed issues](https://github.com/Azure/apiops/issues?q=is%3Aissue+is%3Aclosed) in case your question has already been answered. Don't forget to provide any additional information if needed (e.g. scrubbed logs, detailed feature requests,etc.).
  Whenever it's feasible, please don't hesitate to send a Pull Request (PR) our way. We'd greatly appreciate it, and we'll gladly assess and incorporate your changes.

@jaliyaudagedara
Copy link
Author

jaliyaudagedara commented Aug 12, 2024

@guythetechie, @waelkdouh,

Seems ApiDiagnostic.cs was dropped in version >6.0.0? Any particular reason?

@jaliyaudagedara
Copy link
Author

This seems related: #616

@guythetechie
Copy link
Contributor

Closing in favor of #616. We're adding support back in, accidentally removed it v6.

@guythetechie guythetechie closed this as not planned Won't fix, can't repro, duplicate, stale Aug 13, 2024
@jaliyaudagedara
Copy link
Author

@guythetechie

Thanks for confirmation!

@jaliyaudagedara
Copy link
Author

@guythetechie

Can confirm diagnostics are getting published with v6.0.1.1

@spk2piyu
Copy link

spk2piyu commented Sep 2, 2024

The version v6.0.1.1 supports extraction for api level diagnostics but not replacing loggerId from the configuration..yaml file.

Here is the APIOPS logs for API level diagnostic.
Starting request
2024-09-02T11:25:48.7661199Z Method: PUT
2024-09-02T11:25:48.7662007Z Uri: https://management.azure.com/subscriptions//resourceGroups//providers/Microsoft.ApiManagement/service//apis//diagnostics/applicationinsights?api-version=2023-09-01-preview
2024-09-02T11:25:48.7663677Z Content: {"properties":{"loggerId":"/subscriptions//resourceGroups/<dev RG - it should be target rg>/providers/Microsoft.ApiManagement/service//loggers/","alwaysLog":"allErrors","backend":{"request":{"body":{"bytes":0},"headers":[]},"response":{"body":{"bytes":0},"headers":[]}},"frontend":{"request":{"body":{"bytes":0},"headers":[]},"response":{"body":{"bytes":0},"headers":[]}},"httpCorrelationProtocol":"Legacy","logClientIp":true,"metrics":true,"sampling":{"percentage":100,"samplingType":"fixed"},"verbosity":"verbose"}}

@spk2piyu
Copy link

spk2piyu commented Sep 3, 2024

@guythetechie

Can confirm diagnostics are getting published with v6.0.1.1

Are you able to replace loggerId for the uat and prod (target APIM) from publisher configuration..yaml ?? In my case, it is creating the loggerId with the same name as dev.

Starting request
2024-09-02T11:25:48.7661199Z Method: PUT
2024-09-02T11:25:48.7662007Z Uri: https://management.azure.com/subscriptions//resourceGroups//providers/Microsoft.ApiManagement/service//apis//diagnostics/applicationinsights?api-version=2023-09-01-preview
2024-09-02T11:25:48.7663677Z Content: {"properties":{"loggerId":"/subscriptions//resourceGroups/<dev RG - it should be target rg>/providers/Microsoft.ApiManagement/service//loggers/","alwaysLog":"allErrors","backend":{"request":{"body":{"bytes":0},"headers":[]},"response":{"body":{"bytes":0},"headers":[]}},"frontend":{"request":{"body":{"bytes":0},"headers":[]},"response":{"body":{"bytes":0},"headers":[]}},"httpCorrelationProtocol":"Legacy","logClientIp":true,"metrics":true,"sampling":{"percentage":100,"samplingType":"fixed"},"verbosity":"verbose"}}

@jeroenmaes
Copy link

The version v6.0.1.1 supports extraction for api level diagnostics but not replacing loggerId from the configuration..yaml file.

Here is the APIOPS logs for API level diagnostic. Starting request 2024-09-02T11:25:48.7661199Z Method: PUT 2024-09-02T11:25:48.7662007Z Uri: https://management.azure.com/subscriptions//resourceGroups//providers/Microsoft.ApiManagement/service//apis//diagnostics/applicationinsights?api-version=2023-09-01-preview 2024-09-02T11:25:48.7663677Z Content: {"properties":{"loggerId":"/subscriptions//resourceGroups/<dev RG - it should be target rg>/providers/Microsoft.ApiManagement/service//loggers/","alwaysLog":"allErrors","backend":{"request":{"body":{"bytes":0},"headers":[]},"response":{"body":{"bytes":0},"headers":[]}},"frontend":{"request":{"body":{"bytes":0},"headers":[]},"response":{"body":{"bytes":0},"headers":[]}},"httpCorrelationProtocol":"Legacy","logClientIp":true,"metrics":true,"sampling":{"percentage":100,"samplingType":"fixed"},"verbosity":"verbose"}}

I'm having the exact same experience in my environment.

@guythetechie
Copy link
Contributor

We only support overriding top-level resources in the publisher. You can override service-level diagnostics (which are top-level), but you can't override API diagnostics. In other words, this works:

- diagnostics:
    properties:
      loggerId: abcd

But there is no support for something like this:

- apis
     apiA:
       diagnostics:
         properties:
           loggerId: abcd

We will look into adding support for the above configuration, as we'll need a solution to support overrides in workspaces. In the meantime, you have to update properties.loggerId in apis/apiA/diagnostics/diagnosticA/diagnosticInformation.json with the correct logger.

Also, I don't think the full resource ID of the logger is required in properties.loggerId. You should be able to update it with just /loggers/your-logger-name. This will make sure it works across all APIM instances. It will look for your-logger-name in the current APIM instance instead of the instance specified in /subscriptions/your-subscription/resourceGroups.../original-apim-instance.

@jaliyaudagedara
Copy link
Author

Further @guythetechie s' answer,

Example override yaml,

apimServiceName: <apimServiceName>
namedValues:
  - name: application-insights-instrumentation-key
    properties:
      displayName: application-insights-instrumentation-key
      value: "<application-insights-instrumentation-key>"

loggers:
    - name: appinsights
      properties:
        loggerType: applicationInsights
        description: <description>
        resourceId: "/subscriptions/<subscription>/resourceGroups/<resourceGroup>/providers/microsoft.insights/components/<applicationInsightsInstance>"
        credentials:
          instrumentationKey: "{{application-insights-instrumentation-key}}"
        isBuffered: true

diagnostics:
   - name: applicationinsights
     properties:
       verbosity: Error
       loggerId: "/subscriptions/<subscription>/resourceGroups/<resourceGroup>/providers/Microsoft.ApiManagement/service/<apimServiceName>/loggers/appinsights"

apis:
  - name: <apiName>
    properties:
      serviceUrl: "<serviceUrl>"
    diagnostics:
      - name: applicationinsights
        properties:
          verbosity: information
          loggerId: "/subscriptions/<subscription>/resourceGroups/<resourceGroup>/providers/Microsoft.ApiManagement/service/<apimServiceName>/loggers/appinsights"

@username-anonymous
Copy link

Has this issue been fixed now as i am getting exactly the same error while using v6.0.1.1. Kindly provide the fix if it's working for anyone and configuration.env.yaml file if there is any change in it to make it work.

@jeroenmaes
Copy link

@guythetechie, I'm looking forward to an alternative solution in v6.
This requested behaviour of transforming a nested loggerId worked just find in v5.

@vinilka8
Copy link

vinilka8 commented Sep 23, 2024

We are also seeing this issue, it doesn't override in v6.0.1.1

This snippet doesn't work anymore

apis:
  - name: <apiName>
    properties:
      serviceUrl: "<serviceUrl>"
    diagnostics:
      - name: applicationinsights
        properties:
          verbosity: information
          loggerId: "/subscriptions/<subscription>/resourceGroups/<resourceGroup>/providers/Microsoft.ApiManagement/service/<apimServiceName>/loggers/appinsights"

@jaliyaudagedara
Copy link
Author

@vinilka8,

Just to clarify, loggerId should follow the following template.

loggerId: "/subscriptions/<subscription>/resourceGroups/<resourceGroup>/providers/Microsoft.ApiManagement/service/<apimServiceName>/loggers/<loggerId>"

@vinilka8
Copy link

vinilka8 commented Sep 23, 2024

/subscriptions/f5760094-......7ae94787f/resourceGroups/rg-apim-tst-cc-01/providers/Microsoft.ApiManagement/service/apim-org-tst-cc-01/loggers/appi-apim-tst-cc-01

this what we use, and it's not working

@vinilka8
Copy link

we using this approach for a year, since version 4, we just migrated today from 4 to 6.0.1.1, but it start screaming with an error stating that it can't override, and failed the publisher

@waelkdouh
Copy link
Contributor

Please publish the logs here. Make sure you scrape the logs before you publish though.

@LukaszHryniewiecki
Copy link

I see that there is new version released v 6.0.1.2in release log there is an info that that bug was fixed, however I still see same error diagnostics information for API's are not overwritten from config yaml file.

Example:

  • name: product-cognitive-search-apis
    properties:
    path: "indexes"
    diagnostics:
    • name: applicationinsights
      properties:
      loggerId: "/subscriptions/<subscription_ID}/resourceGroups/test-product-api-00001- rg/providers/Microsoft.ApiManagement/service/test-product-api-01-apim/loggers/dap-api-01-appi"
      verbosity: "information"

I'm using release version for linux in devops pipeline.

@rkosom
Copy link

rkosom commented Dec 10, 2024

I am getting same issue for loggerID , not able to replace using configuration.dev.yaml

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

No branches or pull requests

9 participants