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

AWS creates NaN co2e values when usage type is EUW2-PublicIPv4:InUseAddress #1225

Open
2 tasks done
mgriffin-scottlogic opened this issue Aug 24, 2023 · 0 comments
Open
2 tasks done
Labels
bug Something isn't working seeking contributor Appropriate for an outside contributor to do and requires less context on the ccf methodology

Comments

@mgriffin-scottlogic
Copy link
Contributor

Preflight Checklist

  • I have read the Contributing Guidelines for this project.
  • I have searched the issue tracker for an issue that matches the one I want to file, without success.

Issue Details

  • Operating System:
    • Windows 10 (22H2)
  • Browser:
    • Chrome

Expected Behavior

When dealing with an unknown usage type, the data should still give a valid number for it

Actual Behavior

Co2e for a single row estimate can be a NaN, which then affects any other valid numbers that it is accumulated with

To Reproduce

Could be reproduced via a unit test in CostAndUsageReports.test.ts:

  it('deals with unknown usage types', async () => {
    mockStartQueryExecution(startQueryExecutionResponse)
    mockGetQueryExecution(getQueryExecutionResponse)
    mockGetQueryResults(athenaMockGetQueryResultsWithVpcNan)

    const athenaService = new CostAndUsageReports(
      new ComputeEstimator(),
      new StorageEstimator(AWS_CLOUD_CONSTANTS.SSDCOEFFICIENT),
      new StorageEstimator(AWS_CLOUD_CONSTANTS.HDDCOEFFICIENT),
      new NetworkingEstimator(AWS_CLOUD_CONSTANTS.NETWORKING_COEFFICIENT),
      new MemoryEstimator(AWS_CLOUD_CONSTANTS.MEMORY_COEFFICIENT),
      new UnknownEstimator(AWS_CLOUD_CONSTANTS.ESTIMATE_UNKNOWN_USAGE_BY),
      new EmbodiedEmissionsEstimator(
        AWS_CLOUD_CONSTANTS.SERVER_EXPECTED_LIFESPAN,
      ),
      getServiceWrapper(),
    )

    const result = await athenaService.getEstimates(
      startDate,
      endDate,
      grouping,
    )

    expect(result[0].serviceEstimates[0].co2e).toEqual(0)
  })

using the following data for the query result in athena.fixtures.ts:

const queryResultsDataVpcNan = [
  {
    Data: [
      { VarCharValue: '2022-01-01' },
      { VarCharValue: testAccountId },
      { VarCharValue: 'eu-west-2' },
      { VarCharValue: 'AmazonVPC' },
      { VarCharValue: 'EUW2-PublicIPv4:InUseAddress' },
      { VarCharValue: 'Hrs' },
      { VarCharValue: '' },
      { VarCharValue: '120.328611' },
      { VarCharValue: '0.0' },
      { VarCharValue: '' },
      { VarCharValue: '' },
    ],
  },
]

export const athenaMockGetQueryResultsWithVpcNan: Athena.GetQueryResultsOutput = 
  {
    ResultSet: {
      Rows: [queryResultsHeaders, ...queryResultsDataVpcNan],
    }
  }

Additional Information

Some information on this type of usage can be found here. I have tracked down the areas of code that allow NaN's to make it into results but not sure of the best fix.

  • packages\aws\src\lib\CostAndUsageReports.ts:~ln743 - convertAthenaRowToCostAndUsageReportsRow() allows vCpus to be null when its value is an empty string
  • packages\aws\src\lib\CostAndUsageReportsRow.ts:~ln55 - getVCpuHours() will return NaN when vCpuFromReport is null and instanceType cannot be found
  • packages\core\src\compute\ComputeEstimator.ts:~ln44 - ENERGY_ESTIMATION_FORMULA() will return NaN for estimatedKilowattHours when vCpuHours is NaN
  • packages\aws\src\lib\CostAndUsageReports.ts:~ln265 - getEstimateByUsageUnit() checks eventual result of this kilowattHours NaN value and opts not to use the kilowattHours but uses the co2e, which has been calculated from this and is also NaN

Seems to me that the simplest fix is to also set the co2e to zero when the kilowatt hours are NaN but I don't know if it'd be safer to prevent NaNs from being generated earlier in the process.

@4upz 4upz self-assigned this Aug 24, 2023
@camcash17 camcash17 added bug Something isn't working seeking contributor Appropriate for an outside contributor to do and requires less context on the ccf methodology labels Sep 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working seeking contributor Appropriate for an outside contributor to do and requires less context on the ccf methodology
Projects
Status: 🔖 Ready To Do
Development

No branches or pull requests

3 participants