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

[Infra] Create new formulas for RX and TX metrics #188641

Closed
1 task done
crespocarlos opened this issue Jul 18, 2024 · 9 comments · Fixed by #189281
Closed
1 task done

[Infra] Create new formulas for RX and TX metrics #188641

crespocarlos opened this issue Jul 18, 2024 · 9 comments · Fixed by #189281
Assignees
Labels
bug Fixes for quality problems that affect the customer experience Feature:ObsHosts Hosts feature within Observability Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team

Comments

@crespocarlos
Copy link
Contributor

crespocarlos commented Jul 18, 2024

Summary

Related issue

The current RX and TX metrics used in the Hosts View, Inventory UI and Inventory alert rule is incorrect. (more details #184099)

RX: average(host.network.ingress.bytes) * 8 / (max(metricset.period, kql='host.network.ingress.bytes: *') / 1000)
TX: average(host.network.egress.bytes) * 8 / (max(metricset.period, kql='host.network.egress.bytes: *') / 1000)

The calculated rate is wrong because it uses metricset.period. This setting can be configured with any interval in metricbeat. Therefore, the metric is not necessarily a rate per second.

We need to create a new formula to compute the these metrics.

Lens

  • RX: sum(host.network.ingress.bytes) * 8 without normalizeByUnit parameter
  • TX: sum(host.network.egress.bytes) * 8 without normalizeByUnit parameter

Aggregation

  • RX/ TX (replace rx with tx and its corresponding field)
"rx_sum": {
  "sum": {
    "field": "host.network.ingress.bytes"
  }
},
"rx_min_timestamp": {
  "filter": {
    "exists": {
      "field": "host.network.ingress.bytes"
    }
  },
  "aggs": {
    "value": {
      "min": {
        "field": "@timestamp"
      }
    }
  }
},
"rx_max_timestamp": {
  "filter": {
    "exists": {
      "field": "host.network.ingress.bytes"
    }
  },
  "aggs": {
    "value": {
      "max": {
        "field": "@timestamp"
      }
    }
  }
},
"rx": {
  "bucket_script": {
    "buckets_path": {
      "value": "rx_sum",
      "minTime": "rx_min_timestamp>value",
      "maxTime": "rx_max_timestamp>value"
    },
    "script": {
      "source": "(params.value / ((params.maxTime - params.minTime) / 1000))",
      "lang": "painless"
    },
    "gap_policy": "skip"
  }
}

We need to keep backward compatibility with the existing metrics in the Inventory UI and Inventory alert rule

Label for old metrics: Inbound Traffic (Legacy) / Outbound Traffice(Legacy)
Label for new metrics: Inbound Traffic / Outbound Traffic

Implementation hints

  • In order to maintain backward compatibility, we need to create a new TX and RX metric calculation in metrics_data_access/common/inventory_models/host/metrics/snapshot folder. This folder provides the aggregations used by hosts (table), inventory and alerts
  • Update the hosts endpoint to use the new metric RX and TX calculation in the query
  • Update the RX formula used in Lens
  • Update the TX formula used in Lens

Acceptance Criteria

  • Inventory UI and Inventory alert rule shows both old and new RX and TX metrics
  • Hosts View/Asset Details shows only the new RX and TX metric
  • Current RX and TX labels in Inventory and Alert flyout:
    • Old metrics: Inbound Traffic (Legacy) / Outbound Traffice(Legacy)
    • New metrics: Inbound Traffic / Outbound Traffic
  • Hosts View/Asset Details RX and TX tooltip documentation link should point to the new metrics
  • Hosts View/Asset Details RX and TX tooltip displays the new metric calculation
  • Alerts are fired for both old and new RX and TX metrics

Tasks

Preview Give feedback
  1. dedemorton
@crespocarlos crespocarlos added Feature:ObsHosts Hosts feature within Observability Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team labels Jul 18, 2024
@elasticmachine
Copy link
Contributor

Pinging @elastic/obs-ux-infra_services-team (Team:obs-ux-infra_services)

@crespocarlos
Copy link
Contributor Author

crespocarlos commented Jul 18, 2024

@roshan-elastic we need to define the labels for RX and TX metrics in the inventory and alerts flyout.

Should we update the tooltips too?

@roshan-elastic
Copy link

Hey @crespocarlos - I've updated the source docs for this.

Would you mind updating the formulae in the doc for the new ones (I've commented you directly)?

@smith smith added the bug Fixes for quality problems that affect the customer experience label Jul 18, 2024
@smith
Copy link
Contributor

smith commented Jul 18, 2024

Might there be a better name than "Legacy" for this. "Non-normalized/Normalized" is wrong but something?

@crespocarlos
Copy link
Contributor Author

Might there be a better name than "Legacy" for this. "Non-normalized/Normalized" is wrong but something?

Perhaps "Inbound/Outbount Traffic (per metricset.period)" and "Inbound/Outbount Traffic (per second)"?

@roshan-elastic
Copy link

Yeah fair point on this. I like the suggestions - is there any language we could use which leaves out Elastic terminology?

@jennypavlova
Copy link
Member

Hey @crespocarlos,

I checked the issue description - so we want to replace the lens formulas with only the average with sum part and remove the next part, so we have :

export const rx: LensBaseLayer = {
  label: i18n.translate('xpack.metricsData.assetDetails.formulas.rx', {
    defaultMessage: 'Network Inbound (RX)',
  }),
  value: 'sum(host.network.ingress.bytes) * 8',
  format: 'bits',
  decimals: 1,
};

export const tx: LensBaseLayer = {
  label: i18n.translate('xpack.metricsData.assetDetails.formulas.tx', {
    defaultMessage: 'Network Outbound (TX)',
  }),
  value: 'sum(host.network.egress.bytes) * 8',
  format: 'bits',
  decimals: 1,
};

and for the aggregation in the snapshot folder, we keep the old one (rename to {rx/tx}_legacy or something) and we add the new one from the description and use it in the host (and use both in inventory and alerts), right?

@crespocarlos
Copy link
Contributor Author

crespocarlos commented Jul 24, 2024

Hi @jennypavlova

and for the aggregation in the snapshot folder, we keep the old one (rename to {rx/tx}_legacy or something) and we add the new one from the description and use it in the host (and use both in inventory and alerts), right?

We keep the old ones name untouched, otherwise it will affect existing alerts. The hosts view will only use the new one. Inventory and Alerts will use both.

For the old ones, the only thing that changes is their label

@jennypavlova
Copy link
Member

Hey @crespocarlos, Thanks for clarifying!
I am trying the new aggregation now and I am getting 0s when I use it so I am trying to debug that
Some changes I made:

  • Changed the type as it was set to period
  • Changed the value to make sure that we don't use the same name so I replaced it with min/max

link to the wip rx file for reference.

However, this query returns results so I am wondering if we can use it instead or if we need the filter in this case. It looks more straightforward and I am trying to understand why we are changing it and what is wrong with the one from the description. I will investigate more and try to debug it (please let me know if you have an idea why it's not working or if I am missing something there)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience Feature:ObsHosts Hosts feature within Observability Team:obs-ux-infra_services Observability Infrastructure & Services User Experience Team
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants