Skip to content

[monitor-opentelemetry] fix type checking build error #34297

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

Merged
merged 1 commit into from
May 12, 2025

Conversation

jeremymeng
Copy link
Member

The latest @types/node changes add string[] to the RequestOptions.headers type:

DefinitelyTyped/DefinitelyTyped#72617

nodejs/node#58049

This PR adds a check and narrow the type to http.OutgoingHttpHeaders before accessing user-agent
property. We don't ever set the user agent header using the string[] form so this should be fine.

@Copilot Copilot AI review requested due to automatic review settings May 12, 2025 22:02
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR addresses a type checking build error related to recent changes in @types/node by narrowing the type of headers before accessing the user-agent property.

  • The headers check now excludes cases where headers is a string array.
  • The logic for checking both "User-Agent" and "user-agent" header keys is preserved.
Comments suppressed due to low confidence (1)

sdk/monitor/monitor-opentelemetry/src/utils/common.ts:19

  • [nitpick] Consider using the includes() method (e.g., .toString().includes(...)) for substring checking to improve code readability.
.toString().indexOf("azsdk-js-monitor-opentelemetry-exporter") > -1

The latest @types/node changes add `string[]` to the `RequestOptions.headers` type:

DefinitelyTyped/DefinitelyTyped#72617

nodejs/node#58049

This PR adds a check and narrow the the type to `http.OutgoingHttpHeaders` before accessing user-agent
property. We don't ever set the user agent header using the `string[]` form so this should be fine.
@jeremymeng jeremymeng force-pushed the monitor/ot-build-error-may-25 branch from 761455d to 8e2d876 Compare May 12, 2025 22:07
Copy link
Member

@JacksonWeber JacksonWeber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jeremymeng jeremymeng merged commit 400144c into Azure:main May 12, 2025
14 checks passed
@jeremymeng jeremymeng deleted the monitor/ot-build-error-may-25 branch May 12, 2025 22:36
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 this pull request may close these issues.

2 participants