-
Notifications
You must be signed in to change notification settings - Fork 219
Otel bump v0.115.0 -> v0.124.1 #1694
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
base: main
Are you sure you want to change the base?
Conversation
1ee5ad1
to
a6c0bb6
Compare
1c11a8b
to
c019f00
Compare
c019f00
to
ca0c949
Compare
76e99a6
to
b80859b
Compare
b80859b
to
a5c2769
Compare
plugins/inputs/prometheus/start.go
Outdated
} | ||
for _, sc := range scrapeConfigs { | ||
if sc.ScrapeFallbackProtocol == "" { | ||
sc.ScrapeFallbackProtocol = promconfig.PrometheusText1_0_0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change this to 0_0_4 to be consistent with contrib.
return ContextStatement{ | ||
Context: "metric", | ||
Statements: statements, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
error_mode propagate is the default https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/68e1d6cd94bfca9bdf725327d4221f97ce0e0564/processor/transformprocessor/config.go#L39. Needed to be set because unmarshaling fails.
c648ced
to
10bc9e9
Compare
} | ||
|
||
c := confmap.NewFromStringMap(map[string]any{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changing this to interface{} due to this error that occurs:
unable to unmarshal metric decoration processor: invalid metric_statements type, expected: array, got: [{%!t(string=metric) [%!t(string=set(unit, "unit") where name == "disk_free") %!t(string=set(name, "DISK_FREE") where name == "disk_free") %!t(string=set(name, "gpu-utilization") where name == "nvidia_smi_utilization_gpu") %!t(string=set(unit, "unit") where name == "cpu_usage_idle") %!t(string=set(name, "CPU_USAGE_IDLE") where name == "cpu_usage_idle") %!t(string=set(unit, "unit") where name == "cpu_usage_nice") %!t(string=set(name, "cpu_time_active_renamed") where name == "cpu_time_active")]}]
Unmarshal fails when array of ContextStatement is passed and even when we add error mode to struct of context statement it is expecting an array of interface{} otherwise it fails. This is the pr for adding error_mode: open-telemetry/opentelemetry-collector-contrib#29017 this is the version of the contrib this was added:https://github.com/open-telemetry/opentelemetry-collector-contrib/releases/tag/v0.119.0. It is under enhancement but it still broke our translator.
@@ -228,13 +285,13 @@ func Start(configFilePath string, receiver storage.Appendable, shutDownChan chan | |||
g.Add( | |||
func() error { | |||
// we wait until the config is fully loaded. | |||
level.Info(logger).Log("msg", "start ta manager") | |||
logger.Info("start ta manager") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify logs are not extra verbose and are not getting dumped
translator/translate/otel/processor/metricsdecorator/translator.go
Outdated
Show resolved
Hide resolved
2673714
to
596c49e
Compare
plugins/inputs/prometheus/start.go
Outdated
} | ||
level.Info(logger).Log("msg", "See you next time!") | ||
level.Info(kitLogger).Log("msg", "See you next time!") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we using a different logger here? It looks like it was previously using the same logger as the error log above.
plugins/inputs/prometheus/start.go
Outdated
if err != nil { | ||
return errors.Wrapf(err, "couldn't read configuration file %q", filename) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a change in behavior. Why are we returning the error now? Also, if you want to wrap the error, I would suggest using fmt.Errorf("%w", err)
.
4761826
to
2667580
Compare
…loudwatch-agent into paramadon/otel-bump-0.124.1
2667580
to
bea31b5
Compare
) | ||
|
||
level.Info(logger).Log("msg", fmt.Sprintf("Target Allocator is %t", taManager.enabled)) | ||
//Setup Target Allocator Scrape Post Process Handler | ||
logger.Info("Target Allocator status", "enabled", taManager.enabled) //Setup Target Allocator Scrape Post Process Handler |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: I don't see why we've changed this message.
var zapLevel zapcore.Level | ||
switch level.String() { | ||
case "debug": | ||
zapLevel = zapcore.DebugLevel | ||
case "info": | ||
zapLevel = zapcore.InfoLevel | ||
case "warn": | ||
zapLevel = zapcore.WarnLevel | ||
case "error": | ||
zapLevel = zapcore.ErrorLevel | ||
default: | ||
return nil, fmt.Errorf("invalid log level: %s, defaulting to info", level.String()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Still don't see why this had to change from
zapLevel, err := zapcore.ParseLevel(level.String())
if err != nil {
err = fmt.Errorf("Error parsing level: %v. Defaulting to info.", err)
zapLevel = zapcore.InfoLevel
}
Technically a change in behavior as well since it'll fail to create the logger instead of just setting the log level to info.
tam.logger.Error("target Allocator is not configured properly") | ||
return fmt.Errorf("target Allocator is not configured properly") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of logging and then creating the error, could we just log the error that gets returned by reloadConfigTicker
if it's not nil? It looks to me that if we follow this back up, it's already logged as part of "ta manager stopped".
Description of the issue
This pr is for bumping contrib version from v0.116.0 to 0.124.1
Agent test run: https://github.com/aws/amazon-cloudwatch-agent/actions/runs/15495855178
Contrib pr: amazon-contributing/opentelemetry-collector-contrib#311