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

Pin github.com/mitchellh/mapstructure to v1.5.0 #872

Merged
merged 1 commit into from
Sep 29, 2023

Conversation

sky333999
Copy link
Contributor

@sky333999 sky333999 commented Sep 29, 2023

Description of the issue

With OTel upgrade to v0.84.0, it also upgraded mapstructure to v1.5.1 that brings along a change in behavior where empty slices overwrite the OTel defaults that would have been used when unmarshalling from yaml.
An example is where the yaml generated via config translation typically has service->telemetry->logs->output_paths set to [] and when initializing the OTel collector, the default is ["stderr"].
Post this change, the [] would overwrite the default ["stderr"] thus discarding all logs.

As an example, unmarshalling the following into an OTel.Config with v1.5.0 results in output_paths being set to the default i.e. ["stderr"]. But doing the same with v1.5.1 results in output_paths being set to [].

service:
  telemetry:
    logs:
      output_paths: []
      encoding: console

Description of changes

Pinning mapstructure to the prior version. No major changes between v1.5.0 and v1.5.1

License

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Tests

  • Built images with latest main prior and after this change. I noticed no OTel logging before and I see the logs after.

Requirements

Before commit the code, please do the following steps.

  1. Run make fmt and make fmt-sh
  2. Run make lint

@sky333999 sky333999 requested a review from a team as a code owner September 29, 2023 21:01
@codecov-commenter
Copy link

codecov-commenter commented Sep 29, 2023

Codecov Report

Attention: 37 lines in your changes are missing coverage. Please review.

Comparison is base (96d4763) 57.58% compared to head (bd595a7) 62.44%.
Report is 410 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #872      +/-   ##
==========================================
+ Coverage   57.58%   62.44%   +4.85%     
==========================================
  Files         370      339      -31     
  Lines       17548    17105     -443     
==========================================
+ Hits        10105    10681     +576     
+ Misses       6848     5871     -977     
+ Partials      595      553      -42     
Files Coverage Δ
cfg/commonconfig/commonconfig.go 8.00% <ø> (ø)
...md/amazon-cloudwatch-agent-config-wizard/wizard.go 59.55% <ø> (-8.51%) ⬇️
...amazon-cloudwatch-agent/amazon-cloudwatch-agent.go 2.68% <ø> (ø)
...oudwatch-agent/register_event_logger_notwindows.go 0.00% <ø> (ø)
...-cloudwatch-agent/register_event_logger_windows.go 0.00% <ø> (ø)
cmd/config-translator/translator.go 0.00% <ø> (ø)
cmd/xray-migration/commands_unix.go 42.50% <ø> (ø)
cmd/xray-migration/commands_windows.go 42.50% <ø> (ø)
cmd/xray-migration/xray-migration.go 30.28% <ø> (ø)
handlers/agentinfo/info.go 84.94% <ø> (ø)
... and 22 more

... and 206 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sky333999 sky333999 merged commit 6977931 into main Sep 29, 2023
6 checks passed
@sky333999 sky333999 deleted the sky333999/mapstructure branch September 29, 2023 21:31
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.

4 participants