[connector/servicegraph] Simplify servicegraph connector config arguments#45399
[connector/servicegraph] Simplify servicegraph connector config arguments#45399ptodev wants to merge 4 commits into
Conversation
| if c.StoreExpirationLoop <= 0 { | ||
| return errors.New("`store_expiration_loop` must be positive") | ||
| } | ||
|
|
||
| if c.CacheLoop <= 0 { | ||
| return errors.New("`cache_loop` must be positive") | ||
| } |
There was a problem hiding this comment.
This is not backwards compatible. Should we log a warning instead?
ptodev
force-pushed
the
simplify-defaults
branch
from
e9c9d7b to
b99f2f2
Compare
ptodev
force-pushed
the
simplify-defaults
branch
from
b99f2f2 to
1b905b6
Compare
|
Please add a changelog. |
ptodev
force-pushed
the
simplify-defaults
branch
from
5e389e9 to
3df91ec
Compare
| defaultPeerAttributes = []string{ | ||
| string(conventions.PeerServiceKey), string(conventionsv125.DBNameKey), string(conventionsv128.DBSystemKey), | ||
| } | ||
|
|
||
| defaultDatabaseNameAttributes = []string{string(conventionsv125.DBNameKey)} |
There was a problem hiding this comment.
FYI in factory.go this has been changed to only use semconv v1.25.0 and the keys remain the same, so this is backwards compatible.
|
@atoulme I did not add a changelog entry because this change as a whole is not user facing. The only user facing bit is that StoreExpirationLoop and CacheLoop are now required to be positive. This would be a breaking change and I'm not sure if it's acceptable. I'm waiting for @mapno's feedback on this. If we go ahead with the breaking change I'll add a changelog entry, but if we don't then I don't think we need a changelog entry. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
ptodev
force-pushed
the
simplify-defaults
branch
from
3df91ec to
4b3d1f8
Compare
|
Hi @mwear, @JaredTan95 👋 Does this PR look ok to you? |
|
Hi @mwear, @JaredTan95, @braydonk, would you mind taking a look please? |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Please resolve conflicts, add a changelog and check the build, then mark ready for review again. |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
4 participants
Description
Some config arguments are not being set in the
createDefaultConfig()function, which could create problems for utilities which attempt to infer the default arguments of an OTel Collector component.Testing
Do I need to add more tests?