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.

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

sig-instrumentation: Add charter document #2266

Merged
merged 1 commit into from
Aug 27, 2018

Conversation

brancz
Copy link
Member

@brancz brancz commented Jun 14, 2018

This PR is currently work in progress and meant for sig-instrumentation to iterate on its charter document.

@kubernetes/sig-instrumentation-misc

@k8s-ci-robot k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Jun 14, 2018

### Out of scope

Outline of things that could be confused as falling into this SIG but don't
Copy link

@coffeepac coffeepac Jun 18, 2018

Choose a reason for hiding this comment

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

Topics we don't cover (suggested):

  • event (metric, log) processing
  • prescribing what are error states

Copy link
Contributor

Choose a reason for hiding this comment

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

I think these are reasonable additions.

- Follow [KEP] decision making process

- Test health
- Canonical health of code published to <link to dashboard>

Choose a reason for hiding this comment

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

should this be a link to the testgrid at: https://k8s-testgrid.appspot.com/sig-instrumentation ?


- Test health
- Canonical health of code published to <link to dashboard>
- Consistently broken tests automatically send an alert to <link to google group>

Choose a reason for hiding this comment

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

@pwittrock pwittrock added the committee/steering Denotes an issue or PR intended to be handled by the steering committee. label Jun 21, 2018
@philips
Copy link
Contributor

philips commented Jul 20, 2018

Thank you for your patience as we work to your SIG charter merged.

Through the process of reviewing the first 11 charters the Steering Committee (SC) found a lot of copy/paste language caused by our poorly designed initial charter template. To fix this there is a new SIG charter template which focuses on the main things we want to see in charters: scope and responsibilities.

Please consider updating to this charter template and putting focus into the scope and responsibilities. SC members will focus more on a deep review of scope more than anything else and using this template will help with that focus.

@brancz brancz force-pushed the sig-instrumentation-charter branch from 7f9ff41 to cb4f618 Compare July 26, 2018 08:19
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jul 26, 2018
@brancz
Copy link
Member Author

brancz commented Jul 26, 2018

@coffeepac @philips I updated this PR with the new charter template and incorporated your comments. Let's go through this in today's sig-instrumentation meeting.

Copy link
Contributor

@DirectXMan12 DirectXMan12 left a comment

Choose a reason for hiding this comment

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

comments inline


By SIG Technical Leads

[sig-governance]: sig-governance.md
Copy link
Contributor

Choose a reason for hiding this comment

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

this doesn't end up at the right place

Copy link
Contributor

Choose a reason for hiding this comment

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

(it's a missing page for me)

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, oversight in the template #2423


- Components required for any Kubernetes cluster in regards to observability.
- Interfaces/API definitions required for any Kubernetes cluster in regards to observability.
- Well established but optional components for Kubernetes clusters, if endorsed by member.
Copy link
Contributor

Choose a reason for hiding this comment

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

make this members (plural), and maybe add a note about having multiple people who can maintain.


#### Cross-cutting and Externally Facing Processes

n/a
Copy link
Contributor

Choose a reason for hiding this comment

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

two concerns here:

  • being the "reviewers" for instrumentation (somewhat akin to api-reviewers)
  • being involved in things like node monitoring, where we collaborate with another sig on the details of monitoring parts of kubernetes

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed, I think people should come to this SIG for advice on what metrics to export and the best practices on the dimensions


### Deviations from [sig-governance]

- This SIG does not have Chairs.
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe say "the SIG does not have chairs separately from the technical leads"

Copy link
Contributor

@philips philips left a comment

Choose a reason for hiding this comment

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

Great start! Added some suggestions.


#### Cross-cutting and Externally Facing Processes

n/a
Copy link
Contributor

Choose a reason for hiding this comment

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

agreed, I think people should come to this SIG for advice on what metrics to export and the best practices on the dimensions

### Out of scope

- Processing of signals. For example ingesting metrics, logs, events into external systems.
- Predescribing what error states are.
Copy link
Contributor

Choose a reason for hiding this comment

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

Prescribing? I don't know if predescribing works here. Maybe something like this:

  • Cataloging thresholds or other error states based on instrumentation e.g. alert configurations


## Scope

Covers best practices for cluster observability through metrics, logging, and
Copy link
Contributor

Choose a reason for hiding this comment

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

Scope should be what a SIG owns. So, perhaps:

"Owns best practices for cluster..."


[Link to SIG section in sigs.yaml][sigs.yaml].

#### Code, Binaries and Services
Copy link
Contributor

Choose a reason for hiding this comment

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


Coordinating metric requirements of different SIGs for other components through finding common APIs.

[Link to SIG section in sigs.yaml][sigs.yaml].
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably better to link here since it is generated from sigs.yaml https://github.com/kubernetes/community/tree/master/sig-instrumentation#subprojects

@brancz brancz force-pushed the sig-instrumentation-charter branch from cb4f618 to 78bb874 Compare July 27, 2018 06:34
@brancz
Copy link
Member Author

brancz commented Jul 27, 2018

@coffeepac @DirectXMan12 @philips All comments addressed. I think we're in a good shape. Let me know if there anything else. Removing [WIP] as I'd be happy with this state.

@brancz brancz changed the title [WIP] sig-instrumentation: Add charter document sig-instrumentation: Add charter document Jul 27, 2018
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 27, 2018
@philips
Copy link
Contributor

philips commented Jul 27, 2018

PTAL @derekwaynecarr @spiffxp as the steering committee reviewers.

Copy link
Contributor

@philips philips left a comment

Choose a reason for hiding this comment

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

Looking good. Just want to be concrete on the code/binaries/services list.


- Components required for any Kubernetes cluster in regards to observability.
- Interfaces/API definitions required for any Kubernetes cluster in regards to observability.
- Well established but optional components for Kubernetes clusters, if endorsed by members. Each component must have two or more members as maintainers.
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't understand this line. Can you explain further?

[List of subprojects](https://github.com/kubernetes/community/tree/master/sig-instrumentation#subprojects)

- Components required for any Kubernetes cluster in regards to observability.
- Interfaces/API definitions required for any Kubernetes cluster in regards to observability.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you link to those APIs and Interfaces that this SIG owns?


[List of subprojects](https://github.com/kubernetes/community/tree/master/sig-instrumentation#subprojects)

- Components required for any Kubernetes cluster in regards to observability.
Copy link
Contributor

Choose a reason for hiding this comment

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

What are those components? Are they the subprojects or something else? Can you link to them?

@brancz brancz force-pushed the sig-instrumentation-charter branch 2 times, most recently from c76d863 to bd7b695 Compare July 27, 2018 07:12
@brancz
Copy link
Member Author

brancz commented Jul 27, 2018

@philips I linked out to projects and prospects as examples for each category. Let me know if you like this format or if you have other suggestions.

Copy link

@coffeepac coffeepac left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @brancz !

Copy link
Member

@spiffxp spiffxp left a comment

Choose a reason for hiding this comment

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

Thanks a bunch for using the shorter format.

I hope these don't come across as nitpicky, I'm trying to look for clarity, overlap and gaps. If these questions are better answered through a high bandwidth medium I can be available as well.

## Scope

Owns best practices for cluster observability through metrics, logging, and
events across all Kubernetes components and development of relevant components.
Copy link
Member

Choose a reason for hiding this comment

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

all Kubernetes components

What does this mean? eg:

  • external dependencies: docker, cri-containerd, rioetcd
  • control plane binaries: kube-apiserver, kube-scheduler, kube-controller-manager
  • node binaries: kubelet, kube-proxy
  • addons: dns, ingress, fluentd, etc.

relevant components

What does this mean? I suspect you're trying to imply things like heapster and metrics-server here, but is there a less open ended way of doing so?

Copy link
Member

Choose a reason for hiding this comment

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

events

Needs more clarity, eg:

  • how does this relate to the events API and its use as represented by kubectl get events?
  • how does this related to the event compression algorithm implemented by sig-api-machinery?
  • do you own best practices on the creation of events?
  • how does this relate to the pod lifecycle event generator used by kubelet?
  • how does this relate to the events generated as part of audit logging?

Copy link
Member

Choose a reason for hiding this comment

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

please just strike the term "events" from the charter. it has not been historically the role of this sig and causes confusion.

Copy link
Member

Choose a reason for hiding this comment

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

to clarify more on events:

  • the event API itself is owned by sig-arch imho given its cross-cutting concern; it predates sigs.
  • the client side compression algo was done by me before formation of sig, but its part of client-go so api-machinery owns it now.
  • the event v2 api proposal is grouped under sig-instrumentation at this time, but in my opinion its addressing the api from a scalability perspective.

see: https://github.com/kubernetes/community/blob/f367271772f6616088473a6d99a0f084266d4047/contributors/design-proposals/instrumentation/events-redesign.md

all this to say, events and their best practices are not owned by a single sig imho. its by definition cross-cutting.

Choose a reason for hiding this comment

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

this SIG is mostly concerned with cross-cutting topics and standardizing them. this SIG owns very little by way of implementations in kubernetes and tries to focus on setting best practices.

with that in mind, would this be a good time to formally move best practices for events into this SIG? this would primarily focus on what is a good event, when should it be emitted, how should it be emitted, etc and little with the concrete implementation of those actions.

to be clear, I am not putting a stake in the ground on this, I am exploring a model that fits with what we've been doing with metrics and logging so far.


#### Code, Binaries and Services

[List of subprojects](https://github.com/kubernetes/community/tree/master/sig-instrumentation#subprojects)
Copy link
Member

Choose a reason for hiding this comment

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

I feel like there are many addons missing from this list, eg:

  • cluster-monitoring
  • fluentd-elasticsearch
  • fluentd-gcp
  • metadata-agent
  • prometheus

etc.

Copy link
Member

Choose a reason for hiding this comment

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

@spiffxp i think addons a conflated with a concept of a distribution, and i am not sure this sig should be the owning body for all of those. defer to @brancz for his input, but i think its a grey area.

Copy link
Member

Choose a reason for hiding this comment

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

I think the question for me is whether these are examples of a metrics pipeline, not necessarily a reference impl, and not ownership of vendor-maintained addons. I know there are folks in this sig who are in these OWNERS files

Copy link
Member Author

Choose a reason for hiding this comment

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

None of the above fall into the "metrics pipeline" category per se, a subset of them could be selected to build one. fluentd-gcp and metadata-agent are or at least should be owned by sig-gcp, both depend on external third party services. Regarding the others:

  • cluster-monitoring: is owned by this sig, but deprecated due to heapster deprecation
  • fluentd-elasticsearch: we never discussed this formally, but I believe this is maintained by sig-instrumentation members, and I'm ok with that.
  • prometheus: this is owned and maintained by us, however as its readme says: "This add-on is an experimental configuration of k8s monitoring using Prometheus used for e2e tests."

What do you want to do with this information? Should we add the addons we own in the sigs.yaml? Generally all of these addons fit into the "Well established but optional components or adapters for Kubernetes clusters" category.


### In scope

Coordinating metric requirements of different SIGs for other components through finding common APIs.
Copy link
Member

Choose a reason for hiding this comment

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

just metrics? or also logs and events?

Copy link
Member

Choose a reason for hiding this comment

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

historically to my knowledge, it has just been metrics and not logs or events.

Choose a reason for hiding this comment

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

logging has been a concern owned by this SIG since at least January 2017 and there are e2e tests owned by this sig that predate that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct, but it's wildly disputed what "correct" APIs for logging could and should look like, this goes from logging libraries (glog) to the actual format outputted by these. The only thing about logging that sig-instrumentation somewhat owns is the interface of log file directory structure. It neither owns the actual act of writing the files (typically done by the container runtime) nor does it dictate a format or how logs are to be processed afterwards. I believe we do own and maintain optional addons for this though.


### Out of scope

- Processing of signals. For example ingesting metrics, logs, events into external systems.
Copy link
Member

Choose a reason for hiding this comment

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

It may be helpful to have a counterpart to this in the "in scope" part. This seems to imply to me that if you're not about ingesting, you're about exposing/producing.


[List of subprojects](https://github.com/kubernetes/community/tree/master/sig-instrumentation#subprojects)

- Components required for any Kubernetes cluster in regards to observability. ([kubernetes-incubator/metrics-server](https://github.com/kubernetes-incubator/metrics-server), [kubenernetes/heapster](https://github.com/kubernetes/heapster))
Copy link
Member

Choose a reason for hiding this comment

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

What roles do these fill / functionality do they provide that they are required?

Copy link
Member

Choose a reason for hiding this comment

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

i think this is clear enough, but we could say "core metrics pipeline" that used by core components (scheduler, kubectl, etc.)

[List of subprojects](https://github.com/kubernetes/community/tree/master/sig-instrumentation#subprojects)

- Components required for any Kubernetes cluster in regards to observability. ([kubernetes-incubator/metrics-server](https://github.com/kubernetes-incubator/metrics-server), [kubenernetes/heapster](https://github.com/kubernetes/heapster))
- Interfaces/API definitions required for any Kubernetes cluster in regards to observability. ([kubernetes/metrics](https://github.com/kubernetes/metrics), [kubernetes-incubator/custom-metrics-apiserver](https://github.com/kubernetes-incubator/custom-metrics-apiserver))
Copy link
Member

Choose a reason for hiding this comment

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

Same question here

Copy link
Member

Choose a reason for hiding this comment

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

this is really api extension points for supporting metric sources consumed by core k8s components (i.e. hpa consuming custom metric sources via api) where the metric source itself is not a k8s project (i.e. prometheus, etc.)

- Guidance for instrumentation in order to ensure consistent and high quality instrumentation of core Kubernetes components. This includes:
- Reviewing any instrumentation related changes and additions.
- Guidance on what should be instrumented as well as dimensions of the same.
- Creating, adding and maintaining the Kubernetes instrumentation guidelines.
Copy link
Member

Choose a reason for hiding this comment

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

Do these exist today?

Copy link
Member Author

Choose a reason for hiding this comment

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


- Processing of signals. For example ingesting metrics, logs, events into external systems.
- Dictating what states must result in an alert. Suggestions or opt-in alerts may be in scope.

Copy link
Member

Choose a reason for hiding this comment

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

It may be helpful to specifically call out which kinds of addons you view as out of scope because they are cloud-provider specific (eg: those that rely on stackdriver likely fall to sig-gcp)

Copy link
Member

Choose a reason for hiding this comment

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

i would say alert management is outside the scope entirely?

Choose a reason for hiding this comment

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

we've been discussing have a base set of 'suggested alerts' around core kubernetes functionality. things like 99% API response time, pod startup time and then make it very clear that the actual thresholds need to be tuned by the user but provide some sane defaults. would that be too prescriptive?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, what @coffeepac said is correct. We have been discussing that suggestions for alerts could be something we can own, as that it already falls into most members daily work, and this could allow to centralize suggested alert sets more and better.


### Deviations from [sig-governance]

The SIG does not have Chairs separately from the Technical Leads.
Copy link
Member

Choose a reason for hiding this comment

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

I would rephrase this as "Tech Leads must also fulfill all of the responsibilities of the Chair role as outlined in [sig-governance]"

You may also want to consider whether Tech Leads also doing Chair duties just happens to be the state of today, or something you wish to require of all future Tech Leads

Copy link
Member

@derekwaynecarr derekwaynecarr left a comment

Choose a reason for hiding this comment

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

see comments.

## Scope

Owns best practices for cluster observability through metrics, logging, and
events across all Kubernetes components and development of relevant components.
Copy link
Member

Choose a reason for hiding this comment

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

please just strike the term "events" from the charter. it has not been historically the role of this sig and causes confusion.


### In scope

Coordinating metric requirements of different SIGs for other components through finding common APIs.
Copy link
Member

Choose a reason for hiding this comment

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

historically to my knowledge, it has just been metrics and not logs or events.

## Scope

Owns best practices for cluster observability through metrics, logging, and
events across all Kubernetes components and development of relevant components.
Copy link
Member

Choose a reason for hiding this comment

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

to clarify more on events:

  • the event API itself is owned by sig-arch imho given its cross-cutting concern; it predates sigs.
  • the client side compression algo was done by me before formation of sig, but its part of client-go so api-machinery owns it now.
  • the event v2 api proposal is grouped under sig-instrumentation at this time, but in my opinion its addressing the api from a scalability perspective.

see: https://github.com/kubernetes/community/blob/f367271772f6616088473a6d99a0f084266d4047/contributors/design-proposals/instrumentation/events-redesign.md

all this to say, events and their best practices are not owned by a single sig imho. its by definition cross-cutting.


#### Code, Binaries and Services

[List of subprojects](https://github.com/kubernetes/community/tree/master/sig-instrumentation#subprojects)
Copy link
Member

Choose a reason for hiding this comment

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

@spiffxp i think addons a conflated with a concept of a distribution, and i am not sure this sig should be the owning body for all of those. defer to @brancz for his input, but i think its a grey area.


[List of subprojects](https://github.com/kubernetes/community/tree/master/sig-instrumentation#subprojects)

- Components required for any Kubernetes cluster in regards to observability. ([kubernetes-incubator/metrics-server](https://github.com/kubernetes-incubator/metrics-server), [kubenernetes/heapster](https://github.com/kubernetes/heapster))
Copy link
Member

Choose a reason for hiding this comment

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

i think this is clear enough, but we could say "core metrics pipeline" that used by core components (scheduler, kubectl, etc.)

[List of subprojects](https://github.com/kubernetes/community/tree/master/sig-instrumentation#subprojects)

- Components required for any Kubernetes cluster in regards to observability. ([kubernetes-incubator/metrics-server](https://github.com/kubernetes-incubator/metrics-server), [kubenernetes/heapster](https://github.com/kubernetes/heapster))
- Interfaces/API definitions required for any Kubernetes cluster in regards to observability. ([kubernetes/metrics](https://github.com/kubernetes/metrics), [kubernetes-incubator/custom-metrics-apiserver](https://github.com/kubernetes-incubator/custom-metrics-apiserver))
Copy link
Member

Choose a reason for hiding this comment

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

this is really api extension points for supporting metric sources consumed by core k8s components (i.e. hpa consuming custom metric sources via api) where the metric source itself is not a k8s project (i.e. prometheus, etc.)


- Processing of signals. For example ingesting metrics, logs, events into external systems.
- Dictating what states must result in an alert. Suggestions or opt-in alerts may be in scope.

Copy link
Member

Choose a reason for hiding this comment

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

i would say alert management is outside the scope entirely?

@brancz brancz force-pushed the sig-instrumentation-charter branch from bd7b695 to 6617db1 Compare August 2, 2018 12:02
@brancz
Copy link
Member Author

brancz commented Aug 2, 2018

Added further updates according to the comments. Please take a look and let me know if I should add any more information from comments that I have replied with.

@philips
Copy link
Contributor

philips commented Aug 15, 2018

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 15, 2018
@philips
Copy link
Contributor

philips commented Aug 15, 2018

@spiffxp and @derekwaynecarr if you think the SIG Instrumentation charter now looks good can you please approve?

@brancz
Copy link
Member Author

brancz commented Aug 23, 2018

Does this look good to people now or anything else we should change?

@spiffxp
Copy link
Member

spiffxp commented Aug 23, 2018

looking...

Copy link
Member

@spiffxp spiffxp left a comment

Choose a reason for hiding this comment

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

I have two tiny nits on in-scope/out-of-scope. You've got my attention today, I'll see if we can iterate on this quickly.

## Scope

Owns best practices for cluster observability through metrics and logging
across all Kubernetes components and development of components required for all
Copy link
Member

Choose a reason for hiding this comment

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

I'm still not sure "Kubernetes components" is a well understood, well defined term, but I will assume you are OK deferring the definition of this term to SIG Architecture and accepting whatever scope that implies if the definition is refined.

Copy link
Member

Choose a reason for hiding this comment

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

i am fine w/ that caveat as described by @spiffxp

- Processing of signals. For example ingesting metrics, logs, events into external systems.
- Dictating what states must result in an alert. Suggestions or opt-in alerts may be in scope.
- Cloud provider specific addons are out of scope and should be taken care of by the respective SIG.

Copy link
Member

Choose a reason for hiding this comment

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

I feel like this should contain some of @brancz's response

It neither owns the actual act of writing the files (typically done by the container runtime) nor does it dictate a format or how logs are to be processed afterwards.

eg:

  • The act of writing log files, their format, or how they are to be processed afterwards

coordinates metric requirements of different SIGs for other components through
finding common APIs (such as the resource/core, custom and external metrics
APIs).

Copy link
Member

Choose a reason for hiding this comment

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

This seems relevant as well

The only thing about logging that sig-instrumentation somewhat owns is the interface of log file directory structure.


#### Code, Binaries and Services

[List of subprojects](https://github.com/kubernetes/community/tree/master/sig-instrumentation#subprojects)
Copy link
Member

Choose a reason for hiding this comment

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

Should we add the addons we own in the sigs.yaml?

Yes, I'll open a follow on PR based on your input, it needn't block this

scope, however SIG-Instrumentation is there to advise any contributors with
instrumentation decisions.

As well as giving advise in regards to instrumentation, SIG-Instrumentation
Copy link
Member

Choose a reason for hiding this comment

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

nit: s/advise/advice

@brancz brancz force-pushed the sig-instrumentation-charter branch from 6617db1 to a21a5e9 Compare August 27, 2018 18:50
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 27, 2018
@spiffxp
Copy link
Member

spiffxp commented Aug 27, 2018

/approve

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 27, 2018
@spiffxp
Copy link
Member

spiffxp commented Aug 27, 2018

If I see no objections by end of day I'm going to re-add the LGTM per the above

@philips
Copy link
Contributor

philips commented Aug 27, 2018

sounds good to me @spiffxp

finding common APIs (such as the resource/core, custom and external metrics
APIs).

SIG Instrumentation owns the interface of the log file and directory structure
Copy link
Member

Choose a reason for hiding this comment

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

we did not discuss this in the sig-node charter.

i think this should have some further refinement.

at best I think this is with sig-node, and not exclusively instrumentation. the sig-node charter says "Issues related to node, pod, container monitoring (with sig-instrumentation)" and probably should have also had a line for logging w/ sig-instrumentation. I think this is shared responsibility across the two sigs.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense. Will move that to shared responsibilities.

Copy link
Member Author

@brancz brancz Aug 27, 2018

Choose a reason for hiding this comment

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

I deleted this line and added "The interface of log files and their directory structure written out by container runtimes to be processed by other systems further, is shared responsibility between SIG Node and SIG Instrumentation." To the cross-cutting section.

Copy link
Member

@derekwaynecarr derekwaynecarr left a comment

Choose a reason for hiding this comment

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

i have the one request that we treat the log file and directory structure as a shared responsibility with sig-node, otherwise this is fine w/ me.

@brancz brancz force-pushed the sig-instrumentation-charter branch from a21a5e9 to 7cf8a54 Compare August 27, 2018 23:36
@derekwaynecarr
Copy link
Member

/approve
/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 27, 2018
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: derekwaynecarr, spiffxp

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot merged commit c86d0ab into kubernetes:master Aug 27, 2018
@brancz brancz deleted the sig-instrumentation-charter branch August 27, 2018 23:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. committee/steering Denotes an issue or PR intended to be handled by the steering committee. lgtm "Looks good to me", indicates that a PR is ready to be merged. sig/instrumentation Categorizes an issue or PR as relevant to SIG Instrumentation. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants