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

Inconsistent usage of arn attribute #288

Closed
antoineco opened this issue May 20, 2021 · 3 comments
Closed

Inconsistent usage of arn attribute #288

antoineco opened this issue May 20, 2021 · 3 comments
Assignees
Labels
bug Something isn't working source:Performance Insights

Comments

@antoineco
Copy link
Contributor

antoineco commented May 20, 2021

Problem

The Performance Insights source asks the user to enter an ARN, but this ARN isn't used in the source code. Its only purpose is to carry a region which is used to initialize an API client.

Even more confusing: it's possible to pass an ARN for any AWS service (really any) and the source would still work, casually ignoring the value entered by the user.

spec:
  arn: arn:::us-west-2::  # works (!)

Not only this is a bug, but it also provides a poor user experience, because people are left wondering whether they need to use a PI ARN, a RDS ARN... and the answer is none of them. 🤔

Proposed solution

I see 2 possible ways to fix this:

1. Using an actual Performance Insights ARN

The following page suggests that Performance Insights actually uses ARNs to identify real resources (metrics):

arn:${Partition}:pi:${Region}:${Account}:metrics/${ServiceType}/${Identifier}
-- https://docs.aws.amazon.com/service-authorization/latest/reference/list_awsperformanceinsights.html

spec:
  arn: arn:aws:pi:us-west-2:123456789012:metrics/RDS/db-2JBJ3GC2LEXAMPLEIDENTIFIER
  pollingInterval: 3m
  metricQueries:
  - ...

Pros:

  • Consistent with the rest of our AWS sources
  • Contains both the serviceType and identifier, which are currently separate attributes

Cons:

  • Not particularly user friendly because such ARN can't be obtained via the web console, unlike other AWS products.

2. Replacing the ARN with a "region" attribute

As mentioned in the problem description, even if we don't currently do anything with the provided ARN, we still need users to provide a region.

Tackling this issue could be as easy as renaming "arn" to "region" without any further code change.

spec:
  region: us-west-2
  serviceType: RDS
  identifier: db-2JBJ3GC2LEXAMPLEIDENTIFIER
  pollingInterval: 3m
  metricQueries:
  - ...

Pros:

  • Minimal change in the code
  • No unfamiliar ARN format to look up in a doc in order to use the source

Cons:

  • We use ARNs for almost everything else (which doesn't mean we must be religious about it)
@antoineco antoineco added bug Something isn't working source:Performance Insights labels May 20, 2021
@antoineco antoineco self-assigned this May 20, 2021
@triggermesh triggermesh locked as too heated and limited conversation to collaborators May 20, 2021
@triggermesh triggermesh unlocked this conversation May 20, 2021
@antoineco
Copy link
Contributor Author

I've recreated this issue, this time putting extra care into the description of the issue and the reason why the current API is problematic.

I would be grateful if anyone willing to comment could carefully read the description in its entirety 🙏

@antoineco
Copy link
Contributor Author

Closed by #289

@sebgoa
Copy link
Member

sebgoa commented May 26, 2021

I think the issue was addressed by Jeff. thanks for raising it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working source:Performance Insights
Projects
None yet
Development

No branches or pull requests

2 participants