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

Replace 55681 endpoint with 4318 endpoint for HTTP collector #603

Merged
merged 3 commits into from Jan 24, 2022

Conversation

NathanielRN
Copy link
Contributor

Which problem is this PR solving?

Follow up to core repo PR here: open-telemetry/opentelemetry-js#2395

The specifications just updated the HTTP port to be 4318 in this PR: open-telemetry/opentelemetry-specification#1839

Likewise, the Collector also updated from 55681 to 4318 open-telemetry/opentelemetry-collector#3743, but will support both for some time.

Short description of the changes

  • Updates port for Metrics/Trace HTTP exporter to 4318

@codecov
Copy link

codecov bot commented Aug 4, 2021

Codecov Report

Merging #603 (59529eb) into main (edfc04c) will decrease coverage by 0.58%.
The diff coverage is 100.00%.

❗ Current head 59529eb differs from pull request most recent head 4b1f69b. Consider uploading reports for the commit 4b1f69b to get more accurate results

@@            Coverage Diff             @@
##             main     #603      +/-   ##
==========================================
- Coverage   95.29%   94.71%   -0.59%     
==========================================
  Files          10       16       +6     
  Lines         701      813     +112     
  Branches      142      162      +20     
==========================================
+ Hits          668      770     +102     
- Misses         33       43      +10     
Impacted Files Coverage Δ
...metry-browser-extension-autoinjection/src/types.ts 90.32% <100.00%> (ø)
...er-extension-autoinjection/src/background/index.ts 0.00% <0.00%> (ø)
...ction/src/contentScript/InstrumentationInjector.ts 100.00% <0.00%> (ø)
...njection/src/instrumentation/WebInstrumentation.ts 97.22% <0.00%> (ø)
...extension-autoinjection/src/contentScript/index.ts 0.00% <0.00%> (ø)
...rc/background/ProgrammaticContentScriptInjector.ts 100.00% <0.00%> (ø)

obecny
obecny previously requested changes Aug 6, 2021
Copy link
Member

@obecny obecny left a comment

Choose a reason for hiding this comment

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

we should wait till core repo will change default port and then update the whole contrib - all packages and examples at once after core is released

@github-actions
Copy link
Contributor

github-actions bot commented Nov 1, 2021

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Nov 1, 2021
@rauno56 rauno56 removed the stale label Nov 5, 2021
@github-actions
Copy link
Contributor

github-actions bot commented Jan 5, 2022

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Jan 5, 2022
@blumamir blumamir removed the stale label Jan 11, 2022
@blumamir
Copy link
Member

PR in core repo: open-telemetry/opentelemetry-js#2557

@blumamir
Copy link
Member

open-telemetry/opentelemetry-js#2557

@NathanielRN , now that the change is merged in core repo, can you please solve the conflicts and upgrade the collector image so we can merge this PR?

@vmarchaud vmarchaud merged commit 4de977e into open-telemetry:main Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

core 0.15 - change default port of grpc in graphql example from 55680 to 4317
6 participants