Skip to content

Commit

Permalink
fix(file source, kubernetes_logs source, file sink): make file intern…
Browse files Browse the repository at this point in the history
…al metric tag opt-in (#19145)

* fix(file source, kubernetes_logs source, file sink): make file internal metric tag opt-in

* update cue

* fix tests
  • Loading branch information
dsmith3197 authored and jszwedko committed Nov 16, 2023
1 parent 3f6b237 commit be744f4
Show file tree
Hide file tree
Showing 8 changed files with 31 additions and 28 deletions.
2 changes: 0 additions & 2 deletions docs/DEPRECATIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ For example:

## To be migrated

- file_metric_tag v0.36.0 The `internal_metrics.include_file_tag` should default to `false`.

## To be removed

- http_internal_metrics v0.35.0 `requests_completed_total`, `request_duration_seconds`, and `requests_received_total` internal metrics should be removed.
8 changes: 3 additions & 5 deletions src/internal_events/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,14 @@ use vector_lib::internal_event::{error_stage, error_type};

/// Configuration of internal metrics for file-based components.
#[configurable_component]
#[derive(Clone, Debug, PartialEq, Eq, Derivative)]
#[derivative(Default)]
#[derive(Clone, Debug, PartialEq, Eq, Default)]
#[serde(deny_unknown_fields)]
pub struct FileInternalMetricsConfig {
/// Whether or not to include the "file" tag on the component's corresponding internal metrics.
///
/// This is useful for distinguishing between different files while monitoring. However, the tag's
/// cardinality is unbounded. This defaults to true for now but will default to false in a future version.
#[serde(default = "crate::serde::default_true")]
#[derivative(Default(value = "true"))]
/// cardinality is unbounded.
#[serde(default = "crate::serde::default_false")]
pub include_file_tag: bool,
}

Expand Down
20 changes: 15 additions & 5 deletions src/sinks/file/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -464,7 +464,9 @@ mod tests {
encoding: (None::<FramingConfig>, TextSerializerConfig::default()).into(),
compression: Compression::None,
acknowledgements: Default::default(),
internal_metrics: Default::default(),
internal_metrics: FileInternalMetricsConfig {
include_file_tag: true,
},
};

let (input, _events) = random_lines_with_stream(100, 64, None);
Expand All @@ -487,7 +489,9 @@ mod tests {
encoding: (None::<FramingConfig>, TextSerializerConfig::default()).into(),
compression: Compression::Gzip,
acknowledgements: Default::default(),
internal_metrics: Default::default(),
internal_metrics: FileInternalMetricsConfig {
include_file_tag: true,
},
};

let (input, _) = random_lines_with_stream(100, 64, None);
Expand All @@ -510,7 +514,9 @@ mod tests {
encoding: (None::<FramingConfig>, TextSerializerConfig::default()).into(),
compression: Compression::Zstd,
acknowledgements: Default::default(),
internal_metrics: Default::default(),
internal_metrics: FileInternalMetricsConfig {
include_file_tag: true,
},
};

let (input, _) = random_lines_with_stream(100, 64, None);
Expand Down Expand Up @@ -538,7 +544,9 @@ mod tests {
encoding: (None::<FramingConfig>, TextSerializerConfig::default()).into(),
compression: Compression::None,
acknowledgements: Default::default(),
internal_metrics: Default::default(),
internal_metrics: FileInternalMetricsConfig {
include_file_tag: true,
},
};

let (mut input, _events) = random_events_with_stream(32, 8, None);
Expand Down Expand Up @@ -617,7 +625,9 @@ mod tests {
encoding: (None::<FramingConfig>, TextSerializerConfig::default()).into(),
compression: Compression::None,
acknowledgements: Default::default(),
internal_metrics: Default::default(),
internal_metrics: FileInternalMetricsConfig {
include_file_tag: true,
},
};

let (mut input, _events) = random_lines_with_stream(10, 64, None);
Expand Down
3 changes: 3 additions & 0 deletions src/sources/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -859,6 +859,9 @@ mod tests {
},
data_dir: Some(dir.path().to_path_buf()),
glob_minimum_cooldown_ms: Duration::from_millis(100),
internal_metrics: FileInternalMetricsConfig {
include_file_tag: true,
},
..Default::default()
}
}
Expand Down
14 changes: 4 additions & 10 deletions website/content/en/highlights/2023-12-19-0-35-0-upgrade-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,7 @@ badges:
Vector's 0.35.0 release includes **breaking changes**:

1. [The Throttle transform's `events_discarded_total` internal metric is now opt-in](#events-discarded-total-opt-in)

and **deprecations**:

1. [Deprecation of `file` internal metric tag for file-based components](#deprecate-file-tag)
1. [The `file` internal metric tag is now opt-in for file-based components](#file-tag-opt-in)

and **potentially impactful changes**:

Expand All @@ -37,15 +34,12 @@ potentially unbounded cardinality.

To view events discarded without the `key` tag, use the `component_discarded_events_total` internal metric.

### Deprecations

#### Deprecation of `file` internal metric tag for file-based components {#deprecate-file-tag}
#### The `file` internal metric tag is now opt-in for file-based components {#file-tag-opt-in}

File-based components (file source, Kubernetes logs source, file sink) now include a
`internal_metrics.include_file_tag` config option that determines whether the `file` tag is included on the
component's corresponding internal metrics. This config option defaults to `true` for now to retain the
existing behavior. In the next release, the config option will be updated to default to `false`, as this
`tag` is likely to be of high cardinality.
component's corresponding internal metrics. This config option defaults to `false`, as this `tag` is likely to
be of high cardinality.

### Potentially impactful changes

Expand Down
4 changes: 2 additions & 2 deletions website/cue/reference/components/sinks/base/file.cue
Original file line number Diff line number Diff line change
Expand Up @@ -333,10 +333,10 @@ base: components: sinks: file: configuration: {
Whether or not to include the "file" tag on the component's corresponding internal metrics.
This is useful for distinguishing between different files while monitoring. However, the tag's
cardinality is unbounded. This defaults to true for now but will default to false in a future version.
cardinality is unbounded.
"""
required: false
type: bool: default: true
type: bool: default: false
}
}
path: {
Expand Down
4 changes: 2 additions & 2 deletions website/cue/reference/components/sources/base/file.cue
Original file line number Diff line number Diff line change
Expand Up @@ -213,10 +213,10 @@ base: components: sources: file: configuration: {
Whether or not to include the "file" tag on the component's corresponding internal metrics.
This is useful for distinguishing between different files while monitoring. However, the tag's
cardinality is unbounded. This defaults to true for now but will default to false in a future version.
cardinality is unbounded.
"""
required: false
type: bool: default: true
type: bool: default: false
}
}
line_delimiter: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,10 @@ base: components: sources: kubernetes_logs: configuration: {
Whether or not to include the "file" tag on the component's corresponding internal metrics.
This is useful for distinguishing between different files while monitoring. However, the tag's
cardinality is unbounded. This defaults to true for now but will default to false in a future version.
cardinality is unbounded.
"""
required: false
type: bool: default: true
type: bool: default: false
}
}
kube_config_file: {
Expand Down

0 comments on commit be744f4

Please sign in to comment.