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.

Sign up for GitHub

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

Jump to bottom

enhancement(metrics)!: Updated internal metrics names to match standards #4647

Merged
merged 16 commits into from
Oct 20, 2020
Merged

enhancement(metrics)!: Updated internal metrics names to match standards #4647

merged 16 commits into from
Oct 20, 2020

Conversation

jamtur01
Copy link
Contributor

@jamtur01 jamtur01 commented Oct 19, 2020 •
edited by binarylogic
Loading

Closes #3629

Signed-off-by: James Turnbull james@lovedthanlost.net

@jamtur01
fix(internal_metrics): Updated internal metrics names to match standards
13ef2f5 
Signed-off-by: James Turnbull <james@lovedthanlost.net>
@jamtur01 jamtur01 added domain: data model Anything related to Vector's internal data model domain: metrics Anything related to Vector's metrics events labels Oct 19, 2020
@jamtur01 jamtur01 added this to the 2020-10-12: Son of Flynn milestone Oct 19, 2020
@jamtur01 jamtur01 requested a review from JeanMertz as a code owner October 19, 2020 23:36
@jamtur01 jamtur01 self-assigned this Oct 19, 2020
@jamtur01 jamtur01 changed the title fix(internal_metrics): Updated internal metrics names to match standards fix(metrics): Updated internal metrics names to match standards Oct 19, 2020
@jamtur01
Renamed duplicate_events_discarded to events_discarded
cbc2713 
Signed-off-by: James Turnbull <james@lovedthanlost.net>
@jamtur01
fmt
bbaadfa 
Signed-off-by: James Turnbull <james@lovedthanlost.net>
@jamtur01
Found a few more
7ffd5cc 
Signed-off-by: James Turnbull <james@lovedthanlost.net>
@jamtur01
Removed udp and tcp prefixes
9c904e2 
Signed-off-by: James Turnbull <james@lovedthanlost.net>
@jamtur01
Socket_
c457e3b 
Signed-off-by: James Turnbull <james@lovedthanlost.net>
@jamtur01
Updated vector-api-client version
3d14e8c 
Signed-off-by: James Turnbull <james@lovedthanlost.net>
@jamtur01
Geezus
1803af5 
Signed-off-by: James Turnbull <james@lovedthanlost.net>
@jamtur01
fixed k8s tests
e0a2405 
Signed-off-by: James Turnbull <james@lovedthanlost.net>
jamtur01 and others added 2 commits October 19, 2020 23:41
@jamtur01
Some Ben edits
5d26c5c 
Signed-off-by: James Turnbull <james@lovedthanlost.net>
@binarylogic
Remove uncessary metric tags
2ccaea9 
Signed-off-by: binarylogic <bjohnson@binarylogic.com>
@binarylogic
Add mode tags
364ee47 
Signed-off-by: binarylogic <bjohnson@binarylogic.com>
@binarylogic
Use consistent connection_* names
b95aa53 
Signed-off-by: binarylogic <bjohnson@binarylogic.com>
@binarylogic
Copy link
Contributor

Ha, I was just helping out. I think we got them all.

leebenson
leebenson approved these changes Oct 20, 2020
View reviewed changes
Copy link
Member

@leebenson leebenson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I rejigged some of the GraphQL metrics code in #4652, so I'll probably need to update this further -- but looks good.

MOZGIII
MOZGIII reviewed Oct 20, 2020
View reviewed changes
lib/vector-api-client/Cargo.toml
@@ -1,6 +1,6 @@
[package]
name = "vector-api-client"
version = "0.1.0"
version = "0.1.1"
Copy link
Contributor

@MOZGIII MOZGIII Oct 20, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this version bump is meaningful. I think our consensus for internally used crates is that we don't care about versions yet, as we assume they're only used within our "tiny monorepo".
Versions should probably be given some thought in the near future, as these crates have the potential to be used outside.

jszwedko
jszwedko approved these changes Oct 20, 2020
View reviewed changes
Copy link
Member

@jszwedko jszwedko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I'm happy to see these aligning.

Should we label this as a breaking change? It seems like it could affect people depending on the internal_events component. I realize we haven't publicly documented that component, but it seems to be used by people.

src/internal_events/wasm/compilation.rs
@@ -74,7 +74,7 @@ impl InternalEvent for WasmCompilationProgress {
}

fn emit_metrics(&self) {
counter!("wasm_compilation", 1,
counter!("wasm_compilation_total", 1,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not directly related to this PR (though it could impact the naming), but it is unclear to me exactly what this metric indicates. Does it indicate the count of state transitions?

cc/ @Hoverbear

@binarylogic binarylogic changed the title fix(metrics): Updated internal metrics names to match standards enhancement(metrics)!: Updated internal metrics names to match standards Oct 20, 2020
@jamtur01
Copy link
Contributor Author

@jszwedko I lean against it being a breaking change - it's not released code for which the "API" has no promises of stability.

@jamtur01 jamtur01 merged commit 0e74a64 into master Oct 20, 2020
@jamtur01 jamtur01 deleted the intmetrename branch October 20, 2020 14:22
@jszwedko jszwedko mentioned this pull request Oct 20, 2020
Merged
@leebenson leebenson mentioned this pull request Oct 21, 2020
Merged
@jamtur01 jamtur01 mentioned this pull request Oct 22, 2020
Merged
mengesb pushed a commit to jacobbraaten/vector that referenced this pull request Dec 9, 2020
@jamtur01 @binarylogic
enhancement(metrics)!: Updated internal metrics names to match standa…
71fb442 
…rds (vectordotdev#4647)

Co-authored-by: binarylogic <bjohnson@binarylogic.com>
Signed-off-by: Brian Menges <brian.menges@anaplan.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@binarylogic binarylogic binarylogic approved these changes

@leebenson leebenson leebenson approved these changes

@jszwedko jszwedko jszwedko approved these changes

@MOZGIII MOZGIII MOZGIII approved these changes

@bruceg bruceg Awaiting requested review from bruceg

@JeanMertz JeanMertz Awaiting requested review from JeanMertz

@lukesteensen lukesteensen Awaiting requested review from lukesteensen

Assignees

@jamtur01 jamtur01

Labels
domain: data model Anything related to Vector's internal data model domain: metrics Anything related to Vector's metrics events
Projects
None yet
Milestone
2020-10-12: Son of Flynn
Development

Successfully merging this pull request may close these issues.

Update internal metrics to follow naming rules
5 participants
@jamtur01 @binarylogic @MOZGIII @jszwedko @leebenson