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

setAttribute value should be required #1848

Closed
dyladan opened this issue Jan 20, 2021 · 1 comment · Fixed by #1851
Closed

setAttribute value should be required #1848

dyladan opened this issue Jan 20, 2021 · 1 comment · Fixed by #1851
Assignees

Comments

@dyladan
Copy link
Member

dyladan commented Jan 20, 2021

The span.setAttribute() API could be improved. It tells null and undefined are not allowed for value but type definition has value optional.

/**
* Sets an attribute to the span.
*
* Sets a single Attribute with the key and value passed as arguments.
*
* @param key the key for this attribute.
* @param value the value for this attribute. Setting a value null or
* undefined is invalid and will result in undefined behavior.
*/
setAttribute(key: string, value?: AttributeValue): this;

Originally posted by @Flarna in #1765 (comment)

@dyladan
Copy link
Member Author

dyladan commented Jan 20, 2021

null and undefined is officially undefined behavior according to the spec. We should change the typings to reflect that it is not allowed.

@dyladan dyladan added the up-for-grabs Good for taking. Extra help will be provided by maintainers label Jan 20, 2021
@mwear mwear self-assigned this Jan 20, 2021
@mwear mwear removed the up-for-grabs Good for taking. Extra help will be provided by maintainers label Jan 20, 2021
pichlermarc pushed a commit to dynatrace-oss-contrib/opentelemetry-js that referenced this issue Dec 15, 2023
* feat: add esnext target for auto-instr-web package

* remove unnecessary jsx line from tsconfig

* add skipLibCheck to match tsconfig.esm
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants