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.

Sign up for GitHub

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

Jump to bottom

Conflicting type name in API and experimental logs API #3640

Closed
1 of 2 tasks
Flarna opened this issue Feb 28, 2023 · 2 comments · Fixed by #3641
Closed
1 of 2 tasks

Conflicting type name in API and experimental logs API #3640

Flarna opened this issue Feb 28, 2023 · 2 comments · Fixed by #3641

Comments

@Flarna
Copy link
Member

Flarna commented Feb 28, 2023

It seems the experimental logs API exports types already in use by API.
In special there is LoggerOptions here and here but different.

Currently API LoggerOptions is not exported but there is #3639 which corrects this because it is used in an already exported API.

I guess renaming the API LoggerOptions to DiagLoggerOptions would be a breaking change.

  • This only affects the JavaScript OpenTelemetry library
  • This may affect other libraries, but I would like to get opinions here first
@Flarna Flarna mentioned this issue Feb 28, 2023
Merged
4 tasks
@altinokdarici
Copy link
Contributor

Great catch. I think renaming LoggerOptions to DiagLoggerOptions today wouldn't be a breaking change as it's not yet exported (before completing #3639). I'm ok to hold off #3639 until we rename it.

@altinokdarici altinokdarici mentioned this issue Feb 28, 2023
Merged
4 tasks
@altinokdarici
Copy link
Contributor

@Flarna, I'm addressing this issue in #3641

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Rename Diag/LoggerOptions to DiagLoggerOptions altinokdarici/opentelemetry-js
2 participants
@altinokdarici @Flarna