[datasetexporter]: Upgrade library to 0.17.0 #29446
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
martin-majlis-s1
requested review from
atoulme,
a team,
mx-psi and
dmitryax
as code owners
github-actions
bot
added
cmd/configschema
atoulme
reviewed
Nov 22, 2023
atoulme
approved these changes
Nov 22, 2023
|
I'm worried that these metrics don't look like other metrics exposed by other components of the collector: $ curl localhost:8888/metrics
# HELP otelcol_dataset_eventsEnqueued
# TYPE otelcol_dataset_eventsEnqueued gauge
otelcol_dataset_eventsEnqueued 31
# HELP otelcol_dataset_eventsProcessed
# TYPE otelcol_dataset_eventsProcessed gauge
otelcol_dataset_eventsProcessed 31
(...)
I wasn't able to find any guidelines on defining metrics in components, but looking at other components' metrics, I think the metrics from the DataSet exporter should have the names e.g.:
|
Co-authored-by: Andrzej Stencel <astencel@sumologic.com>
Co-authored-by: Andrzej Stencel <astencel@sumologic.com>
Co-authored-by: Andrzej Stencel <astencel@sumologic.com>
Thanks for the review. From the name - I will solve it differently and introduce those attributes. |
martin-majlis-s1
changed the title
|
@astencel-sumo : I have updated the library - scalyr/dataset-go#63. I have also updated some screenshots in the description to show the functionality. |
|
codeboten
reviewed
Nov 27, 2023
exporter/datasetexporter/config.go
Outdated
| @@ -182,14 +186,15 @@ func (c *Config) Validate() error { | |||
| func (c *Config) String() string { | |||
| s := "" | |||
| s += fmt.Sprintf("%s: %s; ", "DatasetURL", c.DatasetURL) | |||
| s += fmt.Sprintf("%s: %s (%d); ", "APIKey", strings.Repeat("*", len(c.APIKey)), len(c.APIKey)) | |||
There was a problem hiding this comment.
a configopaque.String will by default be replaced with [Redacted], is there a reason to override the behaviour here?
There was a problem hiding this comment.
@codeboten : Thanks. But when I have tried it out - it's returning the real value. I have to use apiKey, _ := c.APIKey.MarshalText() - which is not too much better. I will replace it, so it's more consistent with other places.
--- FAIL: TestConfigString (0.00s)
config_test.go:143:
Error Trace: /Users/martin.majlis/development/scalyr-org/opentelemetry-collector-contrib/exporter/datasetexporter/config_test.go:143
Error: Not equal:
expected: "DatasetURL: https://example.com; APIKey: ****** (6); Debug: true; BufferSettings: {MaxLifetime:123ns GroupBy:[field1 field2] RetryInitialInterval:0s RetryMaxInterval:0s RetryMaxElapsedTime:0s RetryShutdownTimeout:0s}; LogsSettings: {ExportResourceInfo:true ExportResourcePrefix:AAA ExportScopeInfo:true ExportScopePrefix:BBB DecomposeComplexMessageField:true DecomposedComplexMessagePrefix:EEE exportSettings:{ExportSeparator:CCC ExportDistinguishingSuffix:DDD}}; TracesSettings: {exportSettings:{ExportSeparator:TTT ExportDistinguishingSuffix:UUU}}; ServerHostSettings: {UseHostName:false ServerHost:foo-bar}; RetrySettings: {Enabled:true InitialInterval:5s RandomizationFactor:0.5 Multiplier:1.5 MaxInterval:30s MaxElapsedTime:5m0s}; QueueSettings: {Enabled:true NumConsumers:10 QueueSize:1000 StorageID:<nil>}; TimeoutSettings: {Timeout:5s}"
actual : "DatasetURL: https://example.com; APIKey: secret (6); Debug: true; BufferSettings: {MaxLifetime:123ns GroupBy:[field1 field2] RetryInitialInterval:0s RetryMaxInterval:0s RetryMaxElapsedTime:0s RetryShutdownTimeout:0s}; LogsSettings: {ExportResourceInfo:true ExportResourcePrefix:AAA ExportScopeInfo:true ExportScopePrefix:BBB DecomposeComplexMessageField:true DecomposedComplexMessagePrefix:EEE exportSettings:{ExportSeparator:CCC ExportDistinguishingSuffix:DDD}}; TracesSettings: {exportSettings:{ExportSeparator:TTT ExportDistinguishingSuffix:UUU}}; ServerHostSettings: {UseHostName:false ServerHost:foo-bar}; RetrySettings: {Enabled:true InitialInterval:5s RandomizationFactor:0.5 Multiplier:1.5 MaxInterval:30s MaxElapsedTime:5m0s}; QueueSettings: {Enabled:true NumConsumers:10 QueueSize:1000 StorageID:<nil>}; TimeoutSettings: {Timeout:5s}"
Diff:
--- Expected
+++ Actual
@@ -1 +1 @@
-DatasetURL: https://example.com; APIKey: ****** (6); Debug: true; BufferSettings: {MaxLifetime:123ns GroupBy:[field1 field2] RetryInitialInterval:0s RetryMaxInterval:0s RetryMaxElapsedTime:0s RetryShutdownTimeout:0s}; LogsSettings: {ExportResourceInfo:true ExportResourcePrefix:AAA ExportScopeInfo:true ExportScopePrefix:BBB DecomposeComplexMessageField:true DecomposedComplexMessagePrefix:EEE exportSettings:{ExportSeparator:CCC ExportDistinguishingSuffix:DDD}}; TracesSettings: {exportSettings:{ExportSeparator:TTT ExportDistinguishingSuffix:UUU}}; ServerHostSettings: {UseHostName:false ServerHost:foo-bar}; RetrySettings: {Enabled:true InitialInterval:5s RandomizationFactor:0.5 Multiplier:1.5 MaxInterval:30s MaxElapsedTime:5m0s}; QueueSettings: {Enabled:true NumConsumers:10 QueueSize:1000 StorageID:<nil>}; TimeoutSettings: {Timeout:5s}
+DatasetURL: https://example.com; APIKey: secret (6); Debug: true; BufferSettings: {MaxLifetime:123ns GroupBy:[field1 field2] RetryInitialInterval:0s RetryMaxInterval:0s RetryMaxElapsedTime:0s RetryShutdownTimeout:0s}; LogsSettings: {ExportResourceInfo:true ExportResourcePrefix:AAA ExportScopeInfo:true ExportScopePrefix:BBB DecomposeComplexMessageField:true DecomposedComplexMessagePrefix:EEE exportSettings:{ExportSeparator:CCC ExportDistinguishingSuffix:DDD}}; TracesSettings: {exportSettings:{ExportSeparator:TTT ExportDistinguishingSuffix:UUU}}; ServerHostSettings: {UseHostName:false ServerHost:foo-bar}; RetrySettings: {Enabled:true InitialInterval:5s RandomizationFactor:0.5 Multiplier:1.5 MaxInterval:30s MaxElapsedTime:5m0s}; QueueSettings: {Enabled:true NumConsumers:10 QueueSize:1000 StorageID:<nil>}; TimeoutSettings: {Timeout:5s}
Test: TestConfigStringThere was a problem hiding this comment.
But when I have tried it out - it's returning the real value.
This is surprising to me... maybe @mx-psi can help here. Will not block this PR on this though.
|
The failed test is related to k8observer: |
This is what I'm now seeing in the metrics: otelcol_dataset_events_enqueued{entity="logs",name="ds1"} 31
otelcol_dataset_events_enqueued{entity="logs",name="ds2"} 31
otelcol_dataset_events_processed{entity="logs",name="ds1"} 31
otelcol_dataset_events_processed{entity="logs",name="ds2"} 31This is when I have two exporters defined like this: exporters:
dataset/ds1:
api_key: abc123456
dataset_url: http://localhost:1234
dataset/ds2:
api_key: def
dataset_url: http://localhost:5678
service:
pipelines:
logs/p1:
exporters:
- dataset/ds1
- dataset/ds2It's not ideal, but the exporter names are there in some form. If you're prepared to possibly make changes to these metrics in the future than I guess this is a step in the right direction. We should probably also make it easier for components to expose metrics in the right format, and have that documented. |
andrzej-stencel
approved these changes
Nov 28, 2023
andrzej-stencel
added
the
ready to merge
codeboten
approved these changes
Nov 28, 2023
This was referenced Dec 14, 2023
10 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description: Upgrade to new version of the library.
This PR is implementing following issues:
debugoption whethersession_keyis included or notOther change is that fields that are specified as part of the
group_byconfiguration are now transferred as part of the session info.Link to tracking Issue: #27650, #27652
Testing:
docker-compose.yamland in thesrc/otelcollector/otelcol-config.yml:docker-compose.yamlswitch image to the newly build one in step 1docker-compose.yamlenable feature gate for collecting metrics ---feature-gates=telemetry.useOtelForInternalMetricssrc/otelcollector/otelcol-config.ymlenable metrics scraping by prometheussrc/otelcollector/otelcol-config.ymladd configuration for datasetdocker compose up --abort-on-container-exitDocumentation:
Library changes: