refactor(opentelemetry-plugins): move common attributes to core

[rubenvp8510]

Signed-off-by: Ruben Vargas [email protected]

Which problem is this PR solving?

Fixes #522

[dyladan]

Please sign the CLA

[dyladan]

Can you add comments with links to the appropriate semantic convention specs?

export enum CommonAttributeNames {
  // Required on all spans
  COMPONENT = 'component',

  // HTTP Span attributes https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-http.md
  HTTP_METHOD = 'http.method',
  HTTP_URL = 'http.url',
  HTTP_TARGET = 'http.target',
  HTTP_HOST = 'http.host',
  HTTP_SCHEME = 'http.scheme',
  
... and so on

[rubenvp8510]

I signed it

[dyladan]

Think this is generally an improvement, but I think it raises a few other questions:

1. should this be in core or types/api?
2. is there a reason this is an enum and not just an object? objects would allow us to nest attributes and have for instance AttributeNames.db.required.type
3. if we keep this as an enum, should it be a const enum? then it would compile away.

@open-telemetry/javascript-approvers WDYT?

Additionally, I would like to see jsdocs on each one explaining what it is, when it is required, and what its value should be. For instance, in http if you have http.url you don't need http.scheme, http.host, or http.target. Additionally, client and server spans have different required attributes.

[vmarchaud]

@dyladan

1. I believe this should belong to the core package since the BasePlugin is there
2. I prefer to have a enum since currently there are no nested attributes in the spec i believe
3. Sure i would prefer to have it only at compile time

Also agree that jsdoc would be better, an extract of the spec would be enough for me

[obecny]

should this be revisited now and either refactored or closed as we have opentelemetry-semantic-conventions now ?

[markwolff]

should this be revisited now and either refactored or closed as we have opentelemetry-semantic-conventions now ?

@obecny it can be closed