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

[Integration][New Relic] - Add support for service level #840

Merged
merged 23 commits into from
Aug 15, 2024

Conversation

PeyGis
Copy link
Contributor

@PeyGis PeyGis commented Jul 24, 2024

Description

What - Added support for New Relic Service Levels
Why -
How - Used the NerdGraphQL API to query entities of type SERVICE_LEVEL

Type of change

Please leave one option from the following and delete the rest:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • New Integration (non-breaking change which adds a new integration)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Non-breaking change (fix of existing functionality that will not change current behavior)
  • Documentation (added/updated documentation)

Screenshots

Screenshot 2024-07-24 at 9 13 06 PM

API Documentation

Sample response

{
   "serviceLevel":{
      "indicators":[
         {
            "createdAt":1721030473712,
            "createdBy":{
               "email":"[email protected]"
            },
            "description":"Proportion of requests that are served without errors.",
            "guid":"NDM2OTY4MHxFWFR8U0VSVklDRV9MRVZFTHw1OTk0MzM",
            "id":"599433",
            "name":"Port API Stg 01 - Success",
            "objectives":[
               {
                  "description":"None",
                  "name":"None",
                  "target":98.47,
                  "timeWindow":{
                     "rolling":{
                        "count":7,
                        "unit":"DAY"
                     }
                  }
               }
            ],
            "resultQueries":{
               "indicator":{
                  "nrql":"SELECT clamp_max((sum(newrelic.sli.valid) - sum(newrelic.sli.bad)) / sum(newrelic.sli.valid) * 100, 100) AS 'SLI' FROM Metric WHERE entity.guid = 'NDM2OTY4MHxFWFR8U0VSVklDRV9MRVZFTHw1OTk0MzM' UNTIL 2 minutes AGO"
               }
            },
            "updatedAt":"None",
            "updatedBy":"None"
         }
      ]
   },
   "tags":{
      "account":[
         "Account 436968[REDACTED]"
      ],
      "accountId":[
         "436968[REDACTED]"
      ],
      "category":[
         "success"
      ],
      "nr.associatedEntityGuid":[
         "NDM2OTY4MHxBUE18QVBQTElDQVRJT058NTkxMTYyMjE0"
      ],
      "nr.associatedEntityName":[
         "Port API Stg 01"
      ],
      "nr.associatedEntityType":[
         "APM_APPLICATION"
      ],
      "nr.sliComplianceCategory":[
         "Compliant"
      ],
      "nr.sloPeriod":[
         "7d"
      ],
      "nr.sloTarget":[
         "98.47%"
      ],
      "trustedAccountId":[
         "436968[REDACTED]"
      ]
   },
   "__SLI":{
      "SLI":99.92846924177397
   }
}

Comment on lines 168 to 170
"serviceLevelIndicator": {
"type": "number",
"title": "Service Level Indicator"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"serviceLevelIndicator": {
"type": "number",
"title": "Service Level Indicator"
"sli": {
"type": "number",
"title": "SLI"

):
nrql = (
service_level.get("serviceLevel", {})
.get("indicators", [])[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

is it possible that they will have more than one indicator and would want to have more than one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I confirmed this and it is not possible to have more than 1

Comment on lines +3 to +6
actor {
account(id: {{ account_id }}) {
nrql(query: "{{ nrql_query }}") {
results
Copy link
Contributor

Choose a reason for hiding this comment

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

how is that query is specific for SLI, can you add an example of the expected query that will be generated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

"indicator":{
                  "nrql":"SELECT clamp_max((sum(newrelic.sli.valid) - sum(newrelic.sli.bad)) / sum(newrelic.sli.valid) * 100, 100) AS 'SLI' FROM Metric WHERE entity.guid = 'NDM2OTY4MHxFWFR8U0VSVklDRV9MRVZFTHw1OTk0MzM' UNTIL 2 minutes AGO"
               }

Comment on lines +45 to +46
async for service_level in send_paginated_graph_api_request(
self.http_client,
Copy link
Contributor

Choose a reason for hiding this comment

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

if you look inside the send_paginated_graph_api_request it actually already fetched a batch, but returns it item by item, so to make it return faster, and maybe async gather the SLIs for multiple SLOs I would suggest either adjusting the send_paginated_graph_api_request or gathering a size of a batch and then bringing the SLI for each one, and using the stream_async_iterators_tasks to return the results for each one.

Comment on lines 49 to 52
extract_data=self._extract_service_levels,
):
nrql = (
service_level.get("serviceLevel", {})
Copy link
Contributor

Choose a reason for hiding this comment

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

add comment about that you are taking the nrql that the SLI is actually built from and query it to get the result

from newrelic_integration.utils import (
get_port_resource_configuration_by_newrelic_entity_type,
get_port_resource_configuration_by_port_kind,
)

MAX_CONCURRENT_REQUESTS = 10
Copy link
Contributor

Choose a reason for hiding this comment

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

you are using it only by the newRelicServiceLevel so I would give it a more specific name

Comment on lines 91 to 94
@staticmethod
def _format_tags(entity: dict[Any, Any]) -> dict[Any, Any]:
entity["tags"] = {tag["key"]: tag["values"] for tag in entity.get("tags", [])}
return entity
Copy link
Contributor

Choose a reason for hiding this comment

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

this is repeating code we already have in core/entities.py I would move it to core/utils and re-use it in all places

@@ -7,14 +7,54 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

<!-- towncrier release notes start -->

# Port_Ocean 0.1.65 (2024-08-01)
# Port_Ocean 0.1.70 (2024-08-15)
Copy link
Contributor

Choose a reason for hiding this comment

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

## 0.1.70 (2024-08-15)

@Tankilevitch Tankilevitch merged commit 8dc0671 into main Aug 15, 2024
11 checks passed
@Tankilevitch Tankilevitch deleted the newrelic-slo branch August 15, 2024 12:25
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 this pull request may close these issues.

2 participants